Refactor NACK list creation to build the NACK list as packets arrive.

Also fixes a timer bug related to NACKing in the RTP module which could cause packets to only be NACKed twice if there's frequent packet losses.

Note that I decided to remove any selective NACKing for now as I don't think the gain of doing it is big enough compared to the added complexity. The same reasoning for empty packets. None of them will be retransmitted by a smart sender since the sender would know that they aren't needed.

BUG=1420
TEST=video_coding_unittests, vie_auto_test, video_coding_integrationtests, trybots

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

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

webrtc/modules/video_coding/main/source/decoding_state.cc

@@ -24,7 +24,7 @@ VCMDecodingState::VCMDecodingState()
       temporal_id_(kNoTemporalIdx),
       tl0_pic_id_(kNoTl0PicIdx),
       full_sync_(true),
-      init_(true) {}
+      in_initial_state_(true) {}
 
 VCMDecodingState::~VCMDecodingState() {}
 
@@ -36,7 +36,7 @@ void VCMDecodingState::Reset() {
   temporal_id_ = kNoTemporalIdx;
   tl0_pic_id_ = kNoTl0PicIdx;
   full_sync_ = true;
-  init_ = true;
+  in_initial_state_ = true;
 }
 
 uint32_t VCMDecodingState::time_stamp() const {
@@ -49,7 +49,7 @@ uint16_t VCMDecodingState::sequence_num() const {
 
 bool VCMDecodingState::IsOldFrame(const VCMFrameBuffer* frame) const {
   assert(frame != NULL);
-  if (init_)
+  if (in_initial_state_)
     return false;
   return (LatestTimestamp(time_stamp_, frame->TimeStamp(), NULL)
           == time_stamp_);
@@ -57,7 +57,7 @@ bool VCMDecodingState::IsOldFrame(const VCMFrameBuffer* frame) const {
 
 bool VCMDecodingState::IsOldPacket(const VCMPacket* packet) const {
   assert(packet != NULL);
-  if (init_)
+  if (in_initial_state_)
     return false;
   return (LatestTimestamp(time_stamp_, packet->timestamp, NULL)
            == time_stamp_);
@@ -71,7 +71,7 @@ void VCMDecodingState::SetState(const VCMFrameBuffer* frame) {
   picture_id_ = frame->PictureId();
   temporal_id_ = frame->TemporalId();
   tl0_pic_id_ = frame->Tl0PicId();
-  init_ = false;
+  in_initial_state_ = false;
 }
 
 void VCMDecodingState::SetStateOneBack(const VCMFrameBuffer* frame) {
@@ -91,11 +91,11 @@ void VCMDecodingState::SetStateOneBack(const VCMFrameBuffer* frame) {
     else
       tl0_pic_id_ = frame->Tl0PicId() - 1;
   }
-  init_ = false;
+  in_initial_state_ = false;
 }
 
 void VCMDecodingState::UpdateEmptyFrame(const VCMFrameBuffer* frame) {
-  if (ContinuousFrame(frame) && frame->GetState() == kStateEmpty) {
+  if (ContinuousFrame(frame)) {
     time_stamp_ = frame->TimeStamp();
     sequence_num_ = frame->GetHighSeqNum();
   }
@@ -114,8 +114,8 @@ void VCMDecodingState::SetSeqNum(uint16_t new_seq_num) {
   sequence_num_ = new_seq_num;
 }
 
-bool VCMDecodingState::init() const {
+bool VCMDecodingState::in_initial_state() const {
-  return init_;
+  return in_initial_state_;
 }
 
 bool VCMDecodingState::full_sync() const {
@@ -123,7 +123,7 @@ bool VCMDecodingState::full_sync() const {
 }
 
 void VCMDecodingState::UpdateSyncState(const VCMFrameBuffer* frame) {
-  if (init_)
+  if (in_initial_state_)
     return;
   if (frame->TemporalId() == kNoTemporalIdx ||
       frame->Tl0PicId() == kNoTl0PicIdx) {
@@ -151,7 +151,7 @@ bool VCMDecodingState::ContinuousFrame(const VCMFrameBuffer* frame) const {
   // Return true when in initial state.
   // Note that when a method is not applicable it will return false.
   assert(frame != NULL);
-  if (init_)
+  if (in_initial_state_)
     return true;
 
   if (!ContinuousLayer(frame->TemporalId(), frame->Tl0PicId())) {

webrtc/modules/video_coding/main/source/decoding_state.h

@@ -43,7 +43,7 @@ class VCMDecodingState {
   uint32_t time_stamp() const;
   uint16_t sequence_num() const;
   // Return true if at initial state.
-  bool init() const;
+  bool in_initial_state() const;
   // Return true when sync is on - decode all layers.
   bool full_sync() const;
 
@@ -63,7 +63,7 @@ class VCMDecodingState {
   int         temporal_id_;
   int         tl0_pic_id_;
   bool        full_sync_;  // Sync flag when temporal layers are used.
-  bool        init_;
+  bool        in_initial_state_;
 };
 
 }  // namespace webrtc

webrtc/modules/video_coding/main/source/decoding_state_unittest.cc

@@ -23,7 +23,7 @@ namespace webrtc {
 TEST(TestDecodingState, Sanity) {
   VCMDecodingState dec_state;
   dec_state.Reset();
-  EXPECT_TRUE(dec_state.init());
+  EXPECT_TRUE(dec_state.in_initial_state());
   EXPECT_TRUE(dec_state.full_sync());
 }
 

webrtc/modules/video_coding/main/source/frame_buffer.cc

@@ -204,20 +204,6 @@ VCMFrameBuffer::LatestPacketTimeMs() const
     return _latestPacketTimeMs;
 }
 
-// Build hard NACK list:Zero out all entries in list up to and including the
-// (first) entry equal to _lowSeqNum.
-int VCMFrameBuffer::BuildHardNackList(int* list, int num,
-                                      int nack_seq_nums_index) {
-  return _sessionInfo.BuildHardNackList(list, num, nack_seq_nums_index);
-}
-
-// Build selective NACK list: Create a soft (selective) list of entries to zero
-// out up to and including the (first) entry equal to _lowSeqNum.
-int VCMFrameBuffer::BuildSoftNackList(int* list, int num,
-                                      int nack_seq_nums_index, int rttMs) {
-  return _sessionInfo.BuildSoftNackList(list, num, nack_seq_nums_index, rttMs);
-}
-
 void
 VCMFrameBuffer::IncrementNackCount()
 {

webrtc/modules/video_coding/main/source/frame_buffer.h

@@ -64,15 +64,11 @@ public:
     void SetCountedFrame(bool frameCounted);
     bool GetCountedFrame() const;
 
-    // NACK - Building the NACK lists.
+    // Increments a counter to keep track of the number of packets of this frame
-    // Build hard NACK list: Zero out all entries in list up to and including
+    // which were NACKed before they arrived.
-    // _lowSeqNum.
-    int BuildHardNackList(int* list, int num, int nack_seq_nums_index);
-    // Build soft NACK list: Zero out only a subset of the packets, discard
-    // empty packets.
-    int BuildSoftNackList(int* list, int num, int nack_seq_nums_index,
-                          int rttMs);
     void IncrementNackCount();
+    // Returns the number of packets of this frame which were NACKed before they
+    // arrived.
     WebRtc_Word16 GetNackCount() const;
 
     WebRtc_Word64 LatestPacketTimeMs() const;

webrtc/modules/video_coding/main/source/generic_encoder.cc

@@ -237,7 +237,8 @@ VCMEncodedFrameCallback::EncodedBytes()
 }
 
 void
-VCMEncodedFrameCallback::SetMediaOpt(VCMMediaOptimization *mediaOpt)
+VCMEncodedFrameCallback::SetMediaOpt(
+    media_optimization::VCMMediaOptimization *mediaOpt)
 {
     _mediaOpt = mediaOpt;
 }

webrtc/modules/video_coding/main/source/generic_encoder.h

@@ -18,7 +18,9 @@
 namespace webrtc
 {
 
+namespace media_optimization {
 class VCMMediaOptimization;
+}  // namespace media_optimization
 
 /*************************************/
 /* VCMEncodeFrameCallback class     */
@@ -47,7 +49,7 @@ public:
     /**
     * Set media Optimization
     */
-    void SetMediaOpt (VCMMediaOptimization* mediaOpt);
+    void SetMediaOpt (media_optimization::VCMMediaOptimization* mediaOpt);
 
     void SetPayloadType(WebRtc_UWord8 payloadType) { _payloadType = payloadType; };
     void SetCodecType(VideoCodecType codecType) {_codecType = codecType;};
@@ -62,13 +64,13 @@ private:
                                   RTPVideoHeader** rtp);
 
     VCMPacketizationCallback* _sendCallback;
-    VCMMediaOptimization*     _mediaOpt;
+    media_optimization::VCMMediaOptimization* _mediaOpt;
-    WebRtc_UWord32            _encodedBytes;
+    WebRtc_UWord32 _encodedBytes;
-    WebRtc_UWord8             _payloadType;
+    WebRtc_UWord8 _payloadType;
-    VideoCodecType            _codecType;
+    VideoCodecType _codecType;
-    bool                      _internalSource;
+    bool _internalSource;
 #ifdef DEBUG_ENCODER_BIT_STREAM
-    FILE*                     _bitStreamAfterEncoder;
+    FILE* _bitStreamAfterEncoder;
 #endif
 };// end of VCMEncodeFrameCallback class
 

webrtc/modules/video_coding/main/source/jitter_buffer.cc

@@ -21,6 +21,7 @@
 #include "webrtc/modules/video_coding/main/source/packet.h"
 #include "webrtc/system_wrappers/interface/clock.h"
 #include "webrtc/system_wrappers/interface/critical_section_wrapper.h"
+#include "webrtc/system_wrappers/interface/logging.h"
 #include "webrtc/system_wrappers/interface/trace.h"
 
 namespace webrtc {
@@ -61,6 +62,10 @@ class CompleteDecodableKeyFrameCriteria {
   }
 };
 
+bool HasNonEmptyState(VCMFrameBuffer* frame) {
+  return frame->GetState() != kStateEmpty;
+}
+
 VCMJitterBuffer::VCMJitterBuffer(Clock* clock,
                                  int vcm_id,
                                  int receiver_id,
@@ -95,9 +100,8 @@ VCMJitterBuffer::VCMJitterBuffer(Clock* clock,
       nack_mode_(kNoNack),
       low_rtt_nack_threshold_ms_(-1),
       high_rtt_nack_threshold_ms_(-1),
-      nack_seq_nums_internal_(),
+      missing_sequence_numbers_(SequenceNumberLessThan()),
       nack_seq_nums_(),
-      nack_seq_nums_length_(0),
       max_nack_list_size_(0),
       max_packet_age_to_nack_(0),
       waiting_for_key_frame_(false) {
@@ -141,7 +145,6 @@ void VCMJitterBuffer::CopyFrom(const VCMJitterBuffer& rhs) {
     inter_frame_delay_ = rhs.inter_frame_delay_;
     waiting_for_completion_ = rhs.waiting_for_completion_;
     rtt_ms_ = rhs.rtt_ms_;
-    nack_seq_nums_length_ = rhs.nack_seq_nums_length_;
     waiting_for_key_frame_ = rhs.waiting_for_key_frame_;
     first_packet_ = rhs.first_packet_;
     last_decoded_state_ =  rhs.last_decoded_state_;
@@ -150,13 +153,9 @@ void VCMJitterBuffer::CopyFrom(const VCMJitterBuffer& rhs) {
     assert(max_packet_age_to_nack_ == rhs.max_packet_age_to_nack_);
     memcpy(receive_statistics_, rhs.receive_statistics_,
            sizeof(receive_statistics_));
-    nack_seq_nums_internal_.resize(rhs.nack_seq_nums_internal_.size());
-    std::copy(rhs.nack_seq_nums_internal_.begin(),
-              rhs.nack_seq_nums_internal_.end(),
-              nack_seq_nums_internal_.begin());
     nack_seq_nums_.resize(rhs.nack_seq_nums_.size());
-    std::copy(rhs.nack_seq_nums_.begin(), rhs.nack_seq_nums_.end(),
+    missing_sequence_numbers_ = rhs.missing_sequence_numbers_;
-              nack_seq_nums_.begin());
+    latest_received_sequence_number_ = rhs.latest_received_sequence_number_;
     for (int i = 0; i < kMaxNumberOfFrames; i++) {
       if (frame_buffers_[i] != NULL) {
         delete frame_buffers_[i];
@@ -199,7 +198,6 @@ void VCMJitterBuffer::Start() {
   waiting_for_completion_.timestamp = 0;
   waiting_for_completion_.latest_packet_time = -1;
   first_packet_ = true;
-  nack_seq_nums_length_ = 0;
   waiting_for_key_frame_ = false;
   rtt_ms_ = kDefaultRtt;
   num_not_decodable_packets_ = 0;
@@ -254,7 +252,7 @@ void VCMJitterBuffer::Flush() {
   waiting_for_completion_.timestamp = 0;
   waiting_for_completion_.latest_packet_time = -1;
   first_packet_ = true;
-  nack_seq_nums_length_ = 0;
+  missing_sequence_numbers_.clear();
   WEBRTC_TRACE(webrtc::kTraceDebug, webrtc::kTraceVideoCoding,
                VCMId(vcm_id_, receiver_id_), "JB(0x%x): Jitter buffer: flush",
                this);
@@ -351,7 +349,7 @@ int64_t VCMJitterBuffer::NextTimestamp(uint32_t max_wait_time_ms,
   crit_sect_->Enter();
 
   // Finding oldest frame ready for decoder, check sequence number and size.
-  CleanUpOldFrames();
+  CleanUpOldOrEmptyFrames();
 
   FrameList::iterator it = frame_list_.begin();
 
@@ -366,7 +364,7 @@ int64_t VCMJitterBuffer::NextTimestamp(uint32_t max_wait_time_ms,
       }
       crit_sect_->Enter();
 
-      CleanUpOldFrames();
+      CleanUpOldOrEmptyFrames();
       it = frame_list_.begin();
     } else {
       crit_sect_->Enter();
@@ -394,7 +392,7 @@ int64_t VCMJitterBuffer::NextTimestamp(uint32_t max_wait_time_ms,
 bool VCMJitterBuffer::CompleteSequenceWithNextFrame() {
   CriticalSectionScoped cs(crit_sect_);
   // Finding oldest frame ready for decoder, check sequence number and size
-  CleanUpOldFrames();
+  CleanUpOldOrEmptyFrames();
 
   if (frame_list_.empty())
     return true;
@@ -410,7 +408,7 @@ bool VCMJitterBuffer::CompleteSequenceWithNextFrame() {
   }
 
   // See if we have lost a frame before this one.
-  if (last_decoded_state_.init()) {
+  if (last_decoded_state_.in_initial_state()) {
     // Following start, reset or flush -> check for key frame.
     if (oldest_frame->FrameType() != kVideoFrameKey) {
       return false;
@@ -433,9 +431,9 @@ VCMEncodedFrame* VCMJitterBuffer::GetCompleteFrameForDecoding(
 
   crit_sect_->Enter();
 
-  CleanUpOldFrames();
+  CleanUpOldOrEmptyFrames();
 
-  if (last_decoded_state_.init() && WaitForRetransmissions()) {
+  if (last_decoded_state_.in_initial_state() && WaitForRetransmissions()) {
     waiting_for_key_frame_ = true;
   }
 
@@ -462,7 +460,7 @@ VCMEncodedFrame* VCMJitterBuffer::GetCompleteFrameForDecoding(
 
         // Finding oldest frame ready for decoder, but check
         // sequence number and size
-        CleanUpOldFrames();
+        CleanUpOldOrEmptyFrames();
         it = FindOldestCompleteContinuousFrame(false);
         if (it == frame_list_.end()) {
           wait_time_ms = end_wait_time_ms - clock_->TimeInMilliseconds();
@@ -500,7 +498,7 @@ VCMEncodedFrame* VCMJitterBuffer::GetCompleteFrameForDecoding(
 
   oldest_frame->SetState(kStateDecoding);
 
-  CleanUpOldFrames();
+  CleanUpOldOrEmptyFrames();
 
   if (oldest_frame->FrameType() == kVideoFrameKey) {
     waiting_for_key_frame_ = false;
@@ -508,6 +506,7 @@ VCMEncodedFrame* VCMJitterBuffer::GetCompleteFrameForDecoding(
 
   // We have a frame - update decoded state with frame info.
   last_decoded_state_.SetState(oldest_frame);
+  DropPacketsFromNackList(last_decoded_state_.sequence_num());
 
   crit_sect_->Leave();
 
@@ -524,7 +523,7 @@ VCMEncodedFrame* VCMJitterBuffer::GetFrameForDecoding() {
     return GetFrameForDecodingNACK();
   }
 
-  CleanUpOldFrames();
+  CleanUpOldOrEmptyFrames();
 
   if (frame_list_.empty()) {
     return NULL;
@@ -566,7 +565,7 @@ VCMEncodedFrame* VCMJitterBuffer::GetFrameForDecoding() {
   // Set as decoding. Propagates the missing_frame bit.
   oldest_frame->SetState(kStateDecoding);
 
-  CleanUpOldFrames();
+  CleanUpOldOrEmptyFrames();
 
   if (oldest_frame->FrameType() == kVideoFrameKey) {
     waiting_for_key_frame_ = false;
@@ -576,6 +575,7 @@ VCMEncodedFrame* VCMJitterBuffer::GetFrameForDecoding() {
 
   // We have a frame - update decoded state with frame info.
   last_decoded_state_.SetState(oldest_frame);
+  DropPacketsFromNackList(last_decoded_state_.sequence_num());
 
   return oldest_frame;
 }
@@ -608,6 +608,7 @@ int VCMJitterBuffer::GetFrame(const VCMPacket& packet,
     // belongs to a frame with a timestamp equal to the last decoded
     // timestamp.
     last_decoded_state_.UpdateOldPacket(&packet);
+    DropPacketsFromNackList(last_decoded_state_.sequence_num());
 
     if (num_consecutive_old_packets_ > kMaxConsecutiveOldPackets) {
       Flush();
@@ -669,12 +670,17 @@ int64_t VCMJitterBuffer::LastPacketTime(VCMEncodedFrame* frame,
 VCMFrameBufferEnum VCMJitterBuffer::InsertPacket(VCMEncodedFrame* encoded_frame,
                                                  const VCMPacket& packet) {
   assert(encoded_frame);
+  bool request_key_frame = false;
   CriticalSectionScoped cs(crit_sect_);
   int64_t now_ms = clock_->TimeInMilliseconds();
   VCMFrameBufferEnum buffer_return = kSizeError;
   VCMFrameBufferEnum ret = kSizeError;
   VCMFrameBuffer* frame = static_cast<VCMFrameBuffer*>(encoded_frame);
 
+  // If this packet belongs to an old, already decoded frame, we want to update
+  // the last decoded sequence number.
+  last_decoded_state_.UpdateOldPacket(&packet);
+
   // We are keeping track of the first seq num, the latest seq num and
   // the number of wraps to be able to calculate how many packets we expect.
   if (first_packet_) {
@@ -682,6 +688,17 @@ VCMFrameBufferEnum VCMJitterBuffer::InsertPacket(VCMEncodedFrame* encoded_frame,
     // reset the delay estimate.
     inter_frame_delay_.Reset(clock_->TimeInMilliseconds());
     first_packet_ = false;
+    latest_received_sequence_number_ = packet.seqNum;
+  } else {
+    if (IsPacketRetransmitted(packet)) {
+      frame->IncrementNackCount();
+    }
+    if (!UpdateNackList(packet.seqNum)) {
+      LOG_F(LS_INFO) << "Requesting key frame due to flushed NACK list.";
+      request_key_frame = true;
+    }
+    latest_received_sequence_number_ = LatestSequenceNumber(
+        latest_received_sequence_number_, packet.seqNum, NULL);
   }
 
   // Empty packets may bias the jitter estimate (lacking size component),
@@ -703,7 +720,6 @@ VCMFrameBufferEnum VCMJitterBuffer::InsertPacket(VCMEncodedFrame* encoded_frame,
   }
 
   VCMFrameBufferStateEnum state = frame->GetState();
-  last_decoded_state_.UpdateOldPacket(&packet);
   // Insert packet
   // Check for first packet
   // High sequence number will be -1 if neither an empty packet nor
@@ -720,11 +736,6 @@ VCMFrameBufferEnum VCMJitterBuffer::InsertPacket(VCMEncodedFrame* encoded_frame,
   if (buffer_return > 0) {
     incoming_bit_count_ += packet.sizeBytes << 3;
 
-    // Has this packet been nacked or is it about to be nacked?
-    if (IsPacketRetransmitted(packet)) {
-      frame->IncrementNackCount();
-    }
-
     // Insert each frame once on the arrival of the first packet
     // belonging to that frame (media or empty).
     if (state == kStateEmpty && first) {
@@ -769,6 +780,9 @@ VCMFrameBufferEnum VCMJitterBuffer::InsertPacket(VCMEncodedFrame* encoded_frame,
       assert(false && "JitterBuffer::InsertPacket: Undefined value");
     }
   }
+  if (request_key_frame) {
+    ret = kFlushIndicator;
+  }
   return ret;
 }
 
@@ -782,9 +796,10 @@ uint32_t VCMJitterBuffer::EstimatedJitterMs() {
   // Compute RTT multiplier for estimation.
   // low_rtt_nackThresholdMs_ == -1 means no FEC.
   double rtt_mult = 1.0f;
-  if (nack_mode_ == kNackHybrid && (low_rtt_nack_threshold_ms_ >= 0 &&
+  if (low_rtt_nack_threshold_ms_ >= 0 &&
-      static_cast<int>(rtt_ms_) > low_rtt_nack_threshold_ms_)) {
+      static_cast<int>(rtt_ms_) >= low_rtt_nack_threshold_ms_) {
-    // From here we count on FEC.
+    // For RTTs above low_rtt_nack_threshold_ms_ we don't apply extra delay
+    // when waiting for retransmissions.
     rtt_mult = 0.0f;
   }
   return jitter_estimate_.GetJitterEstimate(rtt_mult);
@@ -801,6 +816,9 @@ void VCMJitterBuffer::SetNackMode(VCMNackMode mode,
                                   int high_rtt_nack_threshold_ms) {
   CriticalSectionScoped cs(crit_sect_);
   nack_mode_ = mode;
+  if (mode == kNoNack) {
+    missing_sequence_numbers_.clear();
+  }
   assert(low_rtt_nack_threshold_ms >= -1 && high_rtt_nack_threshold_ms >= -1);
   assert(high_rtt_nack_threshold_ms == -1 ||
          low_rtt_nack_threshold_ms <= high_rtt_nack_threshold_ms);
@@ -812,7 +830,7 @@ void VCMJitterBuffer::SetNackMode(VCMNackMode mode,
   if (rtt_ms_ == kDefaultRtt && high_rtt_nack_threshold_ms_ != -1) {
     rtt_ms_ = 0;
   }
-  if (nack_mode_ == kNoNack) {
+  if (!WaitForRetransmissions()) {
     jitter_estimate_.ResetNackCount();
   }
 }
@@ -826,8 +844,6 @@ void VCMJitterBuffer::SetNackSettings(size_t max_nack_list_size,
   }
   max_nack_list_size_ = max_nack_list_size;
   max_packet_age_to_nack_ = max_packet_age_to_nack;
-  nack_seq_nums_internal_.resize(max_packet_age_to_nack_);
-  std::fill(nack_seq_nums_internal_.begin(), nack_seq_nums_internal_.end(), -1);
   nack_seq_nums_.resize(max_nack_list_size_);
 }
 
@@ -836,230 +852,130 @@ VCMNackMode VCMJitterBuffer::nack_mode() const {
   return nack_mode_;
 }
 
-uint16_t* VCMJitterBuffer::CreateNackList(uint16_t* nack_list_size,
+uint16_t* VCMJitterBuffer::GetNackList(uint16_t* nack_list_size,
-                                          bool* list_extended) {
+                                       bool* request_key_frame) {
-  assert(nack_list_size);
-  assert(list_extended);
-  // TODO(mikhal/stefan): Refactor to use last_decoded_state.
   CriticalSectionScoped cs(crit_sect_);
-  int i = 0;
+  *request_key_frame = false;
-  int32_t low_seq_num = -1;
+  if (nack_mode_ == kNoNack) {
-  int32_t high_seq_num = -1;
-  *list_extended = false;
-
-  // Don't create a NACK list if we won't wait for the retransmitted packets.
-  if (!WaitForRetransmissions()) {
     *nack_list_size = 0;
     return NULL;
   }
-
+  if (last_decoded_state_.in_initial_state()) {
-  // Find the lowest (last decoded) sequence number and
+    const bool have_non_empty_frame = frame_list_.end() != find_if(
-  // the highest (highest sequence number of the newest frame)
+          frame_list_.begin(), frame_list_.end(), HasNonEmptyState);
-  // sequence number. The NACK list is a subset of the range
+    *request_key_frame = have_non_empty_frame;
-  // between those two numbers.
+    *nack_list_size = 0;
-  GetLowHighSequenceNumbers(&low_seq_num, &high_seq_num);
-
-  // Build a list of all sequence numbers we have.
-  if (low_seq_num == -1 || high_seq_num == -1) {
-    // This happens if we lose the first packet, nothing is popped.
-    if (high_seq_num == -1) {
-      // We have not received any packets yet.
-      *nack_list_size = 0;
-    } else {
-      // Signal that we want a key frame request to be sent.
-      *nack_list_size = 0xffff;
-    }
     return NULL;
   }
-
+  if (TooLargeNackList()) {
-  int number_of_seq_num = 0;
+    *request_key_frame = !HandleTooLargeNackList();
-  if (low_seq_num > high_seq_num) {
-    if (low_seq_num - high_seq_num > 0x00ff) {
-      // Wrap.
-      number_of_seq_num = (0xffff - low_seq_num) + high_seq_num + 1;
-    }
-  } else {
-    number_of_seq_num = high_seq_num - low_seq_num;
   }
-
+  unsigned int i = 0;
-  if (number_of_seq_num > max_packet_age_to_nack_) {
+  SequenceNumberSet::iterator it = missing_sequence_numbers_.begin();
-    // Some of the missing packets are too old.
+  for (; it != missing_sequence_numbers_.end(); ++it, ++i) {
-    WEBRTC_TRACE(webrtc::kTraceWarning, webrtc::kTraceVideoCoding,
+    nack_seq_nums_[i] = *it;
-                 VCMId(vcm_id_, receiver_id_),
-                 "Nack list too large, try to find a key frame and restart "
-                 "from seq: %d. Lowest seq in jb %d",
-                 high_seq_num, low_seq_num);
-
-    // This NACK size will trigger a key frame request.
-    bool found_key_frame = false;
-
-    while (number_of_seq_num > max_packet_age_to_nack_) {
-      found_key_frame = RecycleFramesUntilKeyFrame();
-
-      if (!found_key_frame) {
-        break;
-      }
-
-      // Check if we are still missing too old sequence numbers.
-      low_seq_num = -1;
-      high_seq_num = -1;
-      GetLowHighSequenceNumbers(&low_seq_num, &high_seq_num);
-
-      if (high_seq_num == -1) {
-        assert(low_seq_num != -1);  // This should never happen.
-        // We can't calculate the NACK list length.
-        return NULL;
-      }
-
-      number_of_seq_num = 0;
-      if (low_seq_num > high_seq_num) {
-        if (low_seq_num - high_seq_num > 0x00ff) {
-          // wrap
-          number_of_seq_num = (0xffff - low_seq_num) + high_seq_num + 1;
-          high_seq_num = low_seq_num;
-        }
-      } else {
-        number_of_seq_num = high_seq_num - low_seq_num;
-      }
-    }
-
-    if (!found_key_frame) {
-      // Set the last decoded sequence number to current high.
-      // This is to not get a large nack list again right away.
-      last_decoded_state_.SetSeqNum(static_cast<uint16_t>(high_seq_num));
-      // Set to trigger key frame signal.
-      *nack_list_size = 0xffff;
-      *list_extended = true;
-      WEBRTC_TRACE(webrtc::kTraceDebug, webrtc::kTraceVideoCoding, -1,
-                   "\tNo key frame found, request one. last_decoded_seq_num_ "
-                   "%d", last_decoded_state_.sequence_num());
-    } else {
-      // We have cleaned up the jitter buffer and found a key frame.
-      WEBRTC_TRACE(webrtc::kTraceDebug, webrtc::kTraceVideoCoding, -1,
-                   "\tKey frame found. last_decoded_seq_num_ %d",
-                   last_decoded_state_.sequence_num());
-      *nack_list_size = 0;
-    }
-
-    return NULL;
   }
-
+  *nack_list_size = i;
-  uint16_t seq_number_iterator = static_cast<uint16_t>(low_seq_num + 1);
-  for (i = 0; i < number_of_seq_num; i++) {
-    nack_seq_nums_internal_[i] = seq_number_iterator;
-    seq_number_iterator++;
-  }
-  if (number_of_seq_num > 0) {
-    // Now we have a list of all sequence numbers that could have been sent.
-    // Zero out the ones we have received.
-    int nack_seq_nums_index = 0;
-    for (FrameList::iterator it = frame_list_.begin(); it != frame_list_.end();
-        ++it) {
-      // Reaching thus far means we are going to update the NACK list
-      // When in hybrid mode, we use the soft NACKing feature.
-      if (nack_mode_ == kNackHybrid) {
-        nack_seq_nums_index = (*it)->BuildSoftNackList(
-            &nack_seq_nums_internal_[0], number_of_seq_num,
-            nack_seq_nums_index, rtt_ms_);
-      } else {
-        // Used when the frame is being processed by the decoding thread
-        // don't need to use that info in this loop.
-        nack_seq_nums_index = (*it)->BuildHardNackList(
-            &nack_seq_nums_internal_[0], number_of_seq_num,
-            nack_seq_nums_index);
-      }
-    }
-  }
-
-  // Compress the list.
-  int empty_index = -1;
-  for (i = 0; i < number_of_seq_num; i++) {
-    if (nack_seq_nums_internal_[i] == -1 || nack_seq_nums_internal_[i] == -2) {
-      // This is empty.
-      if (empty_index == -1) {
-        // No empty index before, remember this position.
-        empty_index = i;
-      }
-    } else {
-      // This is not empty.
-      if (empty_index == -1) {
-        // No empty index, continue.
-      } else {
-        nack_seq_nums_internal_[empty_index] = nack_seq_nums_internal_[i];
-        nack_seq_nums_internal_[i] = -1;
-        empty_index++;
-      }
-    }
-  }
-
-  if (empty_index == -1) {
-    // No empty.
-    *nack_list_size = number_of_seq_num;
-  } else {
-    *nack_list_size = empty_index;
-  }
-
-  if (*nack_list_size > max_nack_list_size_) {
-    // Too many packets missing. Better to skip ahead to the next key frame or
-    // to request one.
-    bool found_key_frame = RecycleFramesUntilKeyFrame();
-    if (!found_key_frame) {
-      // Set the last decoded sequence number to current high.
-      // This is to not get a large nack list again right away.
-      last_decoded_state_.SetSeqNum(static_cast<uint16_t>(high_seq_num));
-      // Set to trigger key frame signal.
-      *nack_list_size = 0xffff;
-      *list_extended = true;
-      WEBRTC_TRACE(webrtc::kTraceDebug, webrtc::kTraceVideoCoding, -1,
-                   "\tNo key frame found, request one. nack_list_size: "
-                   "%u", *nack_list_size);
-    } else {
-      *nack_list_size = 0;
-    }
-    return NULL;
-  }
-
-  if (*nack_list_size > nack_seq_nums_length_) {
-    // Larger list: NACK list was extended since the last call.
-    *list_extended = true;
-  }
-  for (unsigned int j = 0; j < *nack_list_size; j++) {
-    // Check if the list has been extended since it was last created, i.e,
-    // new items have been added.
-    if (nack_seq_nums_length_ > j && !*list_extended) {
-      unsigned int k = 0;
-      for (k = j; k < nack_seq_nums_length_; k++) {
-        // Found the item in the last list, i.e, no new items found yet.
-        if (nack_seq_nums_[k] ==
-            static_cast<uint16_t>(nack_seq_nums_internal_[j])) {
-          break;
-        }
-      }
-      if (k == nack_seq_nums_length_) {  // New item not found in last list.
-        *list_extended = true;
-      }
-    } else {
-      *list_extended = true;
-    }
-    nack_seq_nums_[j] = static_cast<uint16_t>(nack_seq_nums_internal_[j]);
-  }
-
-  nack_seq_nums_length_ = *nack_list_size;
-
   return &nack_seq_nums_[0];
 }
 
+bool VCMJitterBuffer::UpdateNackList(uint16_t sequence_number) {
+  if (nack_mode_ == kNoNack) {
+    return true;
+  }
+  // We won't build a NACK list until we have decoded a frame since we probably
+  // won't be able to decode them until we've received a complete key frame.
+  if (!last_decoded_state_.in_initial_state()) {
+    // We have decoded at least one frame.
+    // Make sure we don't add packets which are already too old to be decoded.
+    latest_received_sequence_number_ = LatestSequenceNumber(
+        latest_received_sequence_number_,
+        last_decoded_state_.sequence_num(),
+        NULL);
+    bool in_order = LatestSequenceNumber(sequence_number,
+        latest_received_sequence_number_, NULL) == sequence_number;
+    if (in_order) {
+      // Push any missing sequence numbers to the NACK list.
+      for (uint16_t i = latest_received_sequence_number_ + 1;
+          i < sequence_number; ++i) {
+        missing_sequence_numbers_.insert(missing_sequence_numbers_.end(), i);
+      }
+      if (TooLargeNackList() && !HandleTooLargeNackList()) {
+        return false;
+      }
+      if (MissingTooOldPacket(sequence_number) &&
+          !HandleTooOldPackets(sequence_number)) {
+        return false;
+      }
+    } else {
+      missing_sequence_numbers_.erase(sequence_number);
+    }
+  }
+  return true;
+}
+
+bool VCMJitterBuffer::TooLargeNackList() const {
+  return missing_sequence_numbers_.size() > max_nack_list_size_;
+}
+
+bool VCMJitterBuffer::HandleTooLargeNackList() {
+  // Recycle frames until the NACK list is small enough. It is likely cheaper to
+  // request a key frame than to retransmit this many missing packets.
+  LOG_F(LS_INFO) << "NACK list has grown too large: " <<
+      missing_sequence_numbers_.size() << " > " << max_nack_list_size_;
+  bool key_frame_found = false;
+  while (missing_sequence_numbers_.size() > max_nack_list_size_) {
+    key_frame_found = RecycleFramesUntilKeyFrame();
+  }
+  assert(!key_frame_found || missing_sequence_numbers_.empty());
+  return key_frame_found;
+}
+
+bool VCMJitterBuffer::MissingTooOldPacket(
+    uint16_t latest_sequence_number) const {
+  if (missing_sequence_numbers_.empty()) {
+    return false;
+  }
+  const uint16_t age_of_oldest_missing_packet = latest_sequence_number -
+      *missing_sequence_numbers_.begin();
+  // Recycle frames if the NACK list contains too old sequence numbers as
+  // the packets may have already been dropped by the sender.
+  return age_of_oldest_missing_packet > max_packet_age_to_nack_;
+}
+
+bool VCMJitterBuffer::HandleTooOldPackets(uint16_t latest_sequence_number) {
+  bool key_frame_found = false;
+  const uint16_t age_of_oldest_missing_packet = latest_sequence_number -
+      *missing_sequence_numbers_.begin();
+  LOG_F(LS_INFO) << "NACK list contains too old sequence numbers: " <<
+      age_of_oldest_missing_packet << " > " << max_packet_age_to_nack_;
+  while (MissingTooOldPacket(latest_sequence_number)) {
+    key_frame_found = RecycleFramesUntilKeyFrame();
+  }
+  assert(!key_frame_found || missing_sequence_numbers_.empty());
+  return key_frame_found;
+}
+
+void VCMJitterBuffer::DropPacketsFromNackList(
+    uint16_t last_decoded_sequence_number) {
+  // Erase all sequence numbers from the NACK list which we won't need any
+  // longer.
+  missing_sequence_numbers_.erase(missing_sequence_numbers_.begin(),
+                                  missing_sequence_numbers_.upper_bound(
+                                      last_decoded_sequence_number));
+}
+
 int64_t VCMJitterBuffer::LastDecodedTimestamp() const {
   CriticalSectionScoped cs(crit_sect_);
   return last_decoded_state_.time_stamp();
 }
 
 VCMEncodedFrame* VCMJitterBuffer::GetFrameForDecodingNACK() {
-  CleanUpOldFrames();
+  CleanUpOldOrEmptyFrames();
   // First look for a complete continuous__ frame.
   // When waiting for nack, wait for a key frame, if a continuous frame cannot
   // be determined (i.e. initial decoding state).
-  if (last_decoded_state_.init()) {
+  if (last_decoded_state_.in_initial_state()) {
     waiting_for_key_frame_ = true;
   }
   // Allow for a decodable frame when in Hybrid mode.
@@ -1093,7 +1009,7 @@ VCMEncodedFrame* VCMJitterBuffer::GetFrameForDecodingNACK() {
   oldest_frame->SetState(kStateDecoding);
 
   // Clean up old frames and empty frames.
-  CleanUpOldFrames();
+  CleanUpOldOrEmptyFrames();
 
   if (oldest_frame->FrameType() == kVideoFrameKey) {
     waiting_for_key_frame_ = false;
@@ -1101,6 +1017,7 @@ VCMEncodedFrame* VCMJitterBuffer::GetFrameForDecodingNACK() {
 
   // We have a frame - update decoded state with frame info.
   last_decoded_state_.SetState(oldest_frame);
+  DropPacketsFromNackList(last_decoded_state_.sequence_num());
 
   return oldest_frame;
 }
@@ -1166,11 +1083,13 @@ bool VCMJitterBuffer::RecycleFramesUntilKeyFrame() {
     if (it != frame_list_.end() && (*it)->FrameType() == kVideoFrameKey) {
       // Fake the last_decoded_state to match this key frame.
       last_decoded_state_.SetStateOneBack(*it);
+      DropPacketsFromNackList(last_decoded_state_.sequence_num());
       return true;
     }
   }
   waiting_for_key_frame_ = true;
   last_decoded_state_.Reset();  // TODO(mikhal): No sync.
+  missing_sequence_numbers_.clear();
   return false;
 }
 
@@ -1308,7 +1227,7 @@ FrameList::iterator VCMJitterBuffer::FindOldestCompleteContinuousFrame(
 }
 
 // Must be called under the critical section |crit_sect_|.
-void VCMJitterBuffer::CleanUpOldFrames() {
+void VCMJitterBuffer::CleanUpOldOrEmptyFrames() {
   while (frame_list_.size() > 0) {
     VCMFrameBuffer* oldest_frame = frame_list_.front();
     if (oldest_frame->GetState() == kStateEmpty && frame_list_.size() > 1) {
@@ -1322,6 +1241,9 @@ void VCMJitterBuffer::CleanUpOldFrames() {
       break;
     }
   }
+  if (!last_decoded_state_.in_initial_state()) {
+    DropPacketsFromNackList(last_decoded_state_.sequence_num());
+  }
 }
 
 void VCMJitterBuffer::VerifyAndSetPreviousFrameLost(VCMFrameBuffer* frame) {
@@ -1336,14 +1258,8 @@ void VCMJitterBuffer::VerifyAndSetPreviousFrameLost(VCMFrameBuffer* frame) {
 
 // Must be called from within |crit_sect_|.
 bool VCMJitterBuffer::IsPacketRetransmitted(const VCMPacket& packet) const {
-  if (nack_seq_nums_length_ > 0) {
+  return missing_sequence_numbers_.find(packet.seqNum) !=
-    for (unsigned int i = 0; i < nack_seq_nums_length_; i++) {
+      missing_sequence_numbers_.end();
-      if (packet.seqNum == nack_seq_nums_[i]) {
-        return true;
-      }
-    }
-  }
-  return false;
 }
 
 // Must be called under the critical section |crit_sect_|. Should never be
@@ -1430,47 +1346,13 @@ void VCMJitterBuffer::UpdateJitterEstimate(
   }
 }
 
-// Assumed to be called internally from inside a critical section.
-void VCMJitterBuffer::GetLowHighSequenceNumbers(
-    int32_t* low_seq_num, int32_t* high_seq_num) const {
-  assert(low_seq_num);
-  assert(high_seq_num);
-  // TODO(mikhal/stefan): Refactor to use last_decoded_state.
-  int i = 0;
-  int32_t seq_num = -1;
-
-  *high_seq_num = -1;
-  *low_seq_num = -1;
-  if (!last_decoded_state_.init())
-    *low_seq_num = last_decoded_state_.sequence_num();
-
-  // find highest seq numbers
-  for (i = 0; i < max_number_of_frames_; ++i) {
-    seq_num = frame_buffers_[i]->GetHighSeqNum();
-
-    // Ignore free / empty frames
-    VCMFrameBufferStateEnum state = frame_buffers_[i]->GetState();
-
-    if ((kStateFree != state) &&
-        (kStateEmpty != state) &&
-        (kStateDecoding != state) &&
-        seq_num != -1) {
-      bool wrap;
-      *high_seq_num = LatestSequenceNumber(seq_num, *high_seq_num, &wrap);
-    }
-  }
-}
-
 bool VCMJitterBuffer::WaitForRetransmissions() {
   if (nack_mode_ == kNoNack) {
     // NACK disabled -> don't wait for retransmissions.
     return false;
-  } else if (nack_mode_ == kNackInfinite) {
-    // NACK only -> always wait for retransmissions.
-    return true;
   }
-  // Hybrid mode. Evaluate if the RTT is high, and in that case we don't wait
+  // Evaluate if the RTT is higher than |high_rtt_nack_threshold_ms_|, and in
-  // for retransmissions.
+  // that case we don't wait for retransmissions.
   if (high_rtt_nack_threshold_ms_ >= 0 &&
       rtt_ms_ >= static_cast<unsigned int>(high_rtt_nack_threshold_ms_)) {
     return false;

webrtc/modules/video_coding/main/source/jitter_buffer.h

@@ -12,6 +12,7 @@
 #define WEBRTC_MODULES_VIDEO_CODING_MAIN_SOURCE_JITTER_BUFFER_H_
 
 #include <list>
+#include <set>
 #include <vector>
 
 #include "webrtc/modules/interface/module_common_types.h"
@@ -28,7 +29,9 @@
 namespace webrtc {
 
 enum VCMNackMode {
-  kNackInfinite,
+  kNack,
+  // TODO(holmer): There is no longer a hybrid NACK mode. We should remove this
+  // and replace it with a jitter buffer API for setting allowing decode errors.
   kNackHybrid,
   kNoNack
 };
@@ -153,12 +156,40 @@ class VCMJitterBuffer {
   // Returns the current NACK mode.
   VCMNackMode nack_mode() const;
 
-  // Creates a list of missing sequence numbers.
+  // Returns a list of the sequence numbers currently missing.
-  uint16_t* CreateNackList(uint16_t* nack_list_size, bool* list_extended);
+  uint16_t* GetNackList(uint16_t* nack_list_size, bool* request_key_frame);
 
   int64_t LastDecodedTimestamp() const;
 
  private:
+  class SequenceNumberLessThan {
+   public:
+    bool operator() (const uint16_t& sequence_number1,
+                     const uint16_t& sequence_number2) const {
+      if (sequence_number1 == sequence_number2)
+        return false;
+      return LatestSequenceNumber(sequence_number1, sequence_number2, NULL) ==
+          sequence_number2;
+    }
+  };
+  typedef std::set<uint16_t, SequenceNumberLessThan> SequenceNumberSet;
+
+  // Returns true if the NACK list was updated to cover sequence numbers up to
+  // |sequence_number|. If false a key frame is needed to get into a state where
+  // we can continue decoding.
+  bool UpdateNackList(uint16_t sequence_number);
+  bool TooLargeNackList() const;
+  // Returns true if the NACK list was reduced without problem. If false a key
+  // frame is needed to get into a state where we can continue decoding.
+  bool HandleTooLargeNackList();
+  bool MissingTooOldPacket(uint16_t latest_sequence_number) const;
+  // Returns true if the too old packets was successfully removed from the NACK
+  // list. If false, a key frame is needed to get into a state where we can
+  // continue decoding.
+  bool HandleTooOldPackets(uint16_t latest_sequence_number);
+  // Drops all packets in the NACK list up until |last_decoded_sequence_number|.
+  void DropPacketsFromNackList(uint16_t last_decoded_sequence_number);
+
   // In NACK-only mode this function doesn't return or release non-complete
   // frames unless we have a complete key frame. In hybrid mode, we may release
   // "decodable", incomplete frames.
@@ -183,7 +214,7 @@ class VCMJitterBuffer {
   // Can return a decodable, incomplete frame if |enable_decodable| is true.
   FrameList::iterator FindOldestCompleteContinuousFrame(bool enable_decodable);
 
-  void CleanUpOldFrames();
+  void CleanUpOldOrEmptyFrames();
 
   // Sets the "decodable" and "frame loss" flags of a frame depending on which
   // packets have been received and which are missing.
@@ -206,12 +237,6 @@ class VCMJitterBuffer {
                             unsigned int frame_size,
                             bool incomplete_frame);
 
-  // Returns the lowest and highest known sequence numbers, where the lowest is
-  // the last decoded sequence number if a frame has been decoded.
-  // -1 is returned if a sequence number cannot be determined.
-  void GetLowHighSequenceNumbers(int32_t* low_seq_num,
-                                 int32_t* high_seq_num) const;
-
   // Returns true if we should wait for retransmissions, false otherwise.
   bool WaitForRetransmissions();
 
@@ -265,9 +290,9 @@ class VCMJitterBuffer {
   int low_rtt_nack_threshold_ms_;
   int high_rtt_nack_threshold_ms_;
   // Holds the internal NACK list (the missing sequence numbers).
-  std::vector<int> nack_seq_nums_internal_;
+  SequenceNumberSet missing_sequence_numbers_;
+  uint16_t latest_received_sequence_number_;
   std::vector<uint16_t> nack_seq_nums_;
-  unsigned int nack_seq_nums_length_;
   size_t max_nack_list_size_;
   int max_packet_age_to_nack_;  // Measured in sequence numbers.
   bool waiting_for_key_frame_;

webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc

@@ -161,8 +161,8 @@ class TestRunningJitterBuffer : public ::testing::Test {
 
   virtual void SetUp() {
     clock_.reset(new SimulatedClock(0));
-    max_nack_list_size_ = 250;
+    max_nack_list_size_ = 150;
-    oldest_packet_to_nack_ = 450;
+    oldest_packet_to_nack_ = 250;
     jitter_buffer_ = new VCMJitterBuffer(clock_.get(), -1, -1, true);
     stream_generator = new StreamGenerator(0, 0, clock_->TimeInMilliseconds());
     jitter_buffer_->Start();
@@ -203,19 +203,27 @@ class TestRunningJitterBuffer : public ::testing::Test {
     return jitter_buffer_->InsertPacket(frame, packet);
   }
 
-  void InsertFrame(FrameType frame_type) {
+  VCMFrameBufferEnum InsertFrame(FrameType frame_type) {
     stream_generator->GenerateFrame(frame_type,
                                     (frame_type != kFrameEmpty) ? 1 : 0,
                                     (frame_type == kFrameEmpty) ? 1 : 0,
                                     clock_->TimeInMilliseconds());
-    EXPECT_EQ(kFirstPacket, InsertPacketAndPop(0));
+    VCMFrameBufferEnum ret = InsertPacketAndPop(0);
     clock_->AdvanceTimeMilliseconds(kDefaultFramePeriodMs);
+    return ret;
   }
 
-  void InsertFrames(int num_frames, FrameType frame_type) {
+  VCMFrameBufferEnum InsertFrames(int num_frames, FrameType frame_type) {
+    VCMFrameBufferEnum ret_for_all = kNoError;
     for (int i = 0; i < num_frames; ++i) {
-      InsertFrame(frame_type);
+      VCMFrameBufferEnum ret = InsertFrame(frame_type);
+      if (ret < kNoError) {
+        ret_for_all = ret;
+      } else if (ret_for_all >= kNoError) {
+        ret_for_all = ret;
+      }
     }
+    return ret_for_all;
   }
 
   void DropFrame(int num_packets) {
@@ -250,7 +258,7 @@ class TestJitterBufferNack : public TestRunningJitterBuffer {
  protected:
   virtual void SetUp() {
     TestRunningJitterBuffer::SetUp();
-    jitter_buffer_->SetNackMode(kNackInfinite, -1, -1);
+    jitter_buffer_->SetNackMode(kNack, -1, -1);
   }
 
   virtual void TearDown() {
@@ -260,17 +268,17 @@ class TestJitterBufferNack : public TestRunningJitterBuffer {
 
 TEST_F(TestRunningJitterBuffer, TestFull) {
   // Insert a key frame and decode it.
-  InsertFrame(kVideoFrameKey);
+  EXPECT_GE(InsertFrame(kVideoFrameKey), kNoError);
   EXPECT_TRUE(DecodeCompleteFrame());
   DropFrame(1);
   // Fill the jitter buffer.
-  InsertFrames(kMaxNumberOfFrames, kVideoFrameDelta);
+  EXPECT_GE(InsertFrames(kMaxNumberOfFrames, kVideoFrameDelta), kNoError);
   // Make sure we can't decode these frames.
   EXPECT_FALSE(DecodeCompleteFrame());
   // This frame will make the jitter buffer recycle frames until a key frame.
   // Since none is found it will have to wait until the next key frame before
   // decoding.
-  InsertFrame(kVideoFrameDelta);
+  EXPECT_GE(InsertFrame(kVideoFrameDelta), kNoError);
   EXPECT_FALSE(DecodeCompleteFrame());
 }
 
@@ -278,27 +286,39 @@ TEST_F(TestRunningJitterBuffer, TestEmptyPackets) {
   // Make sure a frame can get complete even though empty packets are missing.
   stream_generator->GenerateFrame(kVideoFrameKey, 3, 3,
                                   clock_->TimeInMilliseconds());
+  bool request_key_frame = false;
   EXPECT_EQ(kFirstPacket, InsertPacketAndPop(4));
+  EXPECT_FALSE(request_key_frame);
   EXPECT_EQ(kIncomplete, InsertPacketAndPop(4));
+  EXPECT_FALSE(request_key_frame);
   EXPECT_EQ(kIncomplete, InsertPacketAndPop(0));
+  EXPECT_FALSE(request_key_frame);
   EXPECT_EQ(kIncomplete, InsertPacketAndPop(0));
+  EXPECT_FALSE(request_key_frame);
   EXPECT_EQ(kCompleteSession, InsertPacketAndPop(0));
+  EXPECT_FALSE(request_key_frame);
 }
 
 TEST_F(TestRunningJitterBuffer, JitterEstimateMode) {
+  bool request_key_frame = false;
   // Default value (should be in kLastEstimate mode).
   InsertFrame(kVideoFrameKey);
+  EXPECT_FALSE(request_key_frame);
   InsertFrame(kVideoFrameDelta);
+  EXPECT_FALSE(request_key_frame);
   EXPECT_GT(20u, jitter_buffer_->EstimatedJitterMs());
   // Set kMaxEstimate with a 2 seconds initial delay.
   jitter_buffer_->SetMaxJitterEstimate(2000u);
   EXPECT_EQ(2000u, jitter_buffer_->EstimatedJitterMs());
   InsertFrame(kVideoFrameDelta);
+  EXPECT_FALSE(request_key_frame);
   EXPECT_EQ(2000u, jitter_buffer_->EstimatedJitterMs());
   // Jitter cannot decrease.
   InsertFrames(2, kVideoFrameDelta);
+  EXPECT_FALSE(request_key_frame);
   uint32_t je1 = jitter_buffer_->EstimatedJitterMs();
   InsertFrames(2, kVideoFrameDelta);
+  EXPECT_FALSE(request_key_frame);
   EXPECT_GE(je1, jitter_buffer_->EstimatedJitterMs());
 }
 
@@ -321,8 +341,8 @@ TEST_F(TestRunningJitterBuffer, StatisticsTest) {
   InsertFrame(kVideoFrameDelta);
   InsertFrame(kVideoFrameKey);
   InsertFrame(kVideoFrameDelta);
-  // Decode some of them to make sure the statistics doesn't depend on if frames
+  // Decode some of them to make sure the statistics doesn't depend on frames
-  // are decoded or not.
+  // being decoded.
   EXPECT_TRUE(DecodeCompleteFrame());
   EXPECT_TRUE(DecodeCompleteFrame());
   jitter_buffer_->FrameStatistics(&num_delta_frames, &num_key_frames);
@@ -352,37 +372,41 @@ TEST_F(TestRunningJitterBuffer, StatisticsTest) {
 
 TEST_F(TestJitterBufferNack, TestEmptyPackets) {
   // Make sure empty packets doesn't clog the jitter buffer.
-  jitter_buffer_->SetNackMode(kNackHybrid, kLowRttNackMs, -1);
+  jitter_buffer_->SetNackMode(kNackHybrid, media_optimization::kLowRttNackMs,
-  InsertFrames(kMaxNumberOfFrames, kFrameEmpty);
+                              -1);
+  EXPECT_GE(InsertFrames(kMaxNumberOfFrames, kFrameEmpty), kNoError);
   InsertFrame(kVideoFrameKey);
   EXPECT_TRUE(DecodeCompleteFrame());
 }
 
 TEST_F(TestJitterBufferNack, TestNackTooOldPackets) {
   // Insert a key frame and decode it.
-  InsertFrame(kVideoFrameKey);
+  EXPECT_GE(InsertFrame(kVideoFrameKey), kNoError);
   EXPECT_TRUE(DecodeCompleteFrame());
 
   // Drop one frame and insert |kNackHistoryLength| to trigger NACKing a too
   // old packet.
   DropFrame(1);
   // Insert a frame which should trigger a recycle until the next key frame.
-  InsertFrames(oldest_packet_to_nack_, kVideoFrameDelta);
+  EXPECT_EQ(kFlushIndicator, InsertFrames(oldest_packet_to_nack_,
+                                          kVideoFrameDelta));
   EXPECT_FALSE(DecodeCompleteFrame());
 
   uint16_t nack_list_length = max_nack_list_size_;
-  bool extended;
+  bool request_key_frame = false;
-  uint16_t* nack_list = jitter_buffer_->CreateNackList(&nack_list_length,
+  uint16_t* nack_list = jitter_buffer_->GetNackList(&nack_list_length,
-                                                       &extended);
+                                                    &request_key_frame);
   // Verify that the jitter buffer requests a key frame.
-  EXPECT_TRUE(nack_list_length == 0xffff && nack_list == NULL);
+  EXPECT_TRUE(request_key_frame);
+  EXPECT_TRUE(nack_list == NULL);
+  EXPECT_EQ(0, nack_list_length);
 
-  InsertFrame(kVideoFrameDelta);
+  EXPECT_GE(InsertFrame(kVideoFrameDelta), kNoError);
   // Waiting for a key frame.
   EXPECT_FALSE(DecodeCompleteFrame());
   EXPECT_FALSE(DecodeFrame());
 
-  InsertFrame(kVideoFrameKey);
+  EXPECT_GE(InsertFrame(kVideoFrameKey), kNoError);
   // The next complete continuous frame isn't a key frame, but we're waiting
   // for one.
   EXPECT_FALSE(DecodeCompleteFrame());
@@ -390,30 +414,48 @@ TEST_F(TestJitterBufferNack, TestNackTooOldPackets) {
   EXPECT_TRUE(DecodeFrame());
 }
 
+TEST_F(TestJitterBufferNack, TestNackLargeJitterBuffer) {
+  // Insert a key frame and decode it.
+  EXPECT_GE(InsertFrame(kVideoFrameKey), kNoError);
+  EXPECT_TRUE(DecodeCompleteFrame());
+
+  // Insert a frame which should trigger a recycle until the next key frame.
+  EXPECT_GE(InsertFrames(oldest_packet_to_nack_, kVideoFrameDelta), kNoError);
+
+  uint16_t nack_list_length = max_nack_list_size_;
+  bool request_key_frame = false;
+  jitter_buffer_->GetNackList(&nack_list_length, &request_key_frame);
+  // Verify that the jitter buffer does not request a key frame.
+  EXPECT_FALSE(request_key_frame);
+  // Verify that no packets are NACKed.
+  EXPECT_EQ(0, nack_list_length);
+  // Verify that we can decode the next frame.
+  EXPECT_TRUE(DecodeCompleteFrame());
+}
+
 TEST_F(TestJitterBufferNack, TestNackListFull) {
   // Insert a key frame and decode it.
-  InsertFrame(kVideoFrameKey);
+  EXPECT_GE(InsertFrame(kVideoFrameKey), kNoError);
   EXPECT_TRUE(DecodeCompleteFrame());
 
   // Generate and drop |kNackHistoryLength| packets to fill the NACK list.
   DropFrame(max_nack_list_size_);
   // Insert a frame which should trigger a recycle until the next key frame.
-  InsertFrame(kVideoFrameDelta);
+  EXPECT_EQ(kFlushIndicator, InsertFrame(kVideoFrameDelta));
   EXPECT_FALSE(DecodeCompleteFrame());
 
   uint16_t nack_list_length = max_nack_list_size_;
-  bool extended;
+  bool request_key_frame = false;
-  uint16_t* nack_list = jitter_buffer_->CreateNackList(&nack_list_length,
+  jitter_buffer_->GetNackList(&nack_list_length, &request_key_frame);
-                                                       &extended);
   // Verify that the jitter buffer requests a key frame.
-  EXPECT_TRUE(nack_list_length == 0xffff && nack_list == NULL);
+  EXPECT_TRUE(request_key_frame);
 
-  InsertFrame(kVideoFrameDelta);
+  EXPECT_GE(InsertFrame(kVideoFrameDelta), kNoError);
   // Waiting for a key frame.
   EXPECT_FALSE(DecodeCompleteFrame());
   EXPECT_FALSE(DecodeFrame());
 
-  InsertFrame(kVideoFrameKey);
+  EXPECT_GE(InsertFrame(kVideoFrameKey), kNoError);
   // The next complete continuous frame isn't a key frame, but we're waiting
   // for one.
   EXPECT_FALSE(DecodeCompleteFrame());
@@ -424,19 +466,21 @@ TEST_F(TestJitterBufferNack, TestNackListFull) {
 TEST_F(TestJitterBufferNack, TestNackBeforeDecode) {
   DropFrame(10);
   // Insert a frame and try to generate a NACK list. Shouldn't get one.
-  InsertFrame(kVideoFrameDelta);
+  EXPECT_GE(InsertFrame(kVideoFrameDelta), kNoError);
   uint16_t nack_list_size = 0;
-  bool extended = false;
+  bool request_key_frame = false;
-  uint16_t* list = jitter_buffer_->CreateNackList(&nack_list_size, &extended);
+  uint16_t* list = jitter_buffer_->GetNackList(&nack_list_size,
+                                                  &request_key_frame);
   // No list generated, and a key frame request is signaled.
   EXPECT_TRUE(list == NULL);
-  EXPECT_EQ(0xFFFF, nack_list_size);
+  EXPECT_EQ(0, nack_list_size);
+  EXPECT_TRUE(request_key_frame);
 }
 
 TEST_F(TestJitterBufferNack, TestNormalOperation) {
-  EXPECT_EQ(kNackInfinite, jitter_buffer_->nack_mode());
+  EXPECT_EQ(kNack, jitter_buffer_->nack_mode());
 
-  InsertFrame(kVideoFrameKey);
+  EXPECT_GE(InsertFrame(kVideoFrameKey), kNoError);
   EXPECT_TRUE(DecodeFrame());
 
   //  ----------------------------------------------------------------
@@ -449,18 +493,20 @@ TEST_F(TestJitterBufferNack, TestNormalOperation) {
   // Verify that the frame is incomplete.
   EXPECT_FALSE(DecodeCompleteFrame());
   while (stream_generator->PacketsRemaining() > 1) {
-    if (stream_generator->NextSequenceNumber() % 10 != 0)
+    if (stream_generator->NextSequenceNumber() % 10 != 0) {
       EXPECT_EQ(kIncomplete, InsertPacketAndPop(0));
-    else
+    } else {
       stream_generator->NextPacket(NULL);  // Drop packet
+    }
   }
   EXPECT_EQ(kIncomplete, InsertPacketAndPop(0));
   EXPECT_EQ(0, stream_generator->PacketsRemaining());
   EXPECT_FALSE(DecodeCompleteFrame());
   EXPECT_FALSE(DecodeFrame());
   uint16_t nack_list_size = 0;
-  bool extended = false;
+  bool request_key_frame = false;
-  uint16_t* list = jitter_buffer_->CreateNackList(&nack_list_size, &extended);
+  uint16_t* list = jitter_buffer_->GetNackList(&nack_list_size,
+                                               &request_key_frame);
   // Verify the NACK list.
   const int kExpectedNackSize = 9;
   ASSERT_EQ(kExpectedNackSize, nack_list_size);
@@ -469,28 +515,33 @@ TEST_F(TestJitterBufferNack, TestNormalOperation) {
 }
 
 TEST_F(TestJitterBufferNack, TestNormalOperationWrap) {
+  bool request_key_frame = false;
   //  -------   ------------------------------------------------------------
   // | 65532 | | 65533 | 65534 | 65535 | x | 1 | .. | 9 | x | 11 |.....| 96 |
   //  -------   ------------------------------------------------------------
   stream_generator->Init(65532, 0, clock_->TimeInMilliseconds());
   InsertFrame(kVideoFrameKey);
+  EXPECT_FALSE(request_key_frame);
   EXPECT_TRUE(DecodeCompleteFrame());
   stream_generator->GenerateFrame(kVideoFrameDelta, 100, 0,
                                   clock_->TimeInMilliseconds());
   EXPECT_EQ(kFirstPacket, InsertPacketAndPop(0));
   while (stream_generator->PacketsRemaining() > 1) {
-    if (stream_generator->NextSequenceNumber() % 10 != 0)
+    if (stream_generator->NextSequenceNumber() % 10 != 0) {
       EXPECT_EQ(kIncomplete, InsertPacketAndPop(0));
-    else
+      EXPECT_FALSE(request_key_frame);
+    } else {
       stream_generator->NextPacket(NULL);  // Drop packet
+    }
   }
   EXPECT_EQ(kIncomplete, InsertPacketAndPop(0));
+  EXPECT_FALSE(request_key_frame);
   EXPECT_EQ(0, stream_generator->PacketsRemaining());
   EXPECT_FALSE(DecodeCompleteFrame());
   EXPECT_FALSE(DecodeCompleteFrame());
   uint16_t nack_list_size = 0;
   bool extended = false;
-  uint16_t* list = jitter_buffer_->CreateNackList(&nack_list_size, &extended);
+  uint16_t* list = jitter_buffer_->GetNackList(&nack_list_size, &extended);
   // Verify the NACK list.
   const int kExpectedNackSize = 10;
   ASSERT_EQ(kExpectedNackSize, nack_list_size);

webrtc/modules/video_coding/main/source/media_opt_util.cc

@@ -23,6 +23,7 @@
 #include "modules/video_coding/main/source/nack_fec_tables.h"
 
 namespace webrtc {
+namespace media_optimization {
 
 VCMProtectionMethod::VCMProtectionMethod():
 _effectivePacketLoss(0),
@@ -952,4 +953,5 @@ VCMLossProtectionLogic::Release()
     _selectedMethod = NULL;
 }
 
-}
+}  // namespace media_optimization
+}  // namespace webrtc

webrtc/modules/video_coding/main/source/media_opt_util.h

@@ -20,8 +20,9 @@
 #include "webrtc/system_wrappers/interface/trace.h"
 #include "webrtc/typedefs.h"
 
-namespace webrtc
+namespace webrtc {
-{
+namespace media_optimization {
+
 // Number of time periods used for (max) window filter for packet loss
 // TODO (marpan): set reasonable window size for filtered packet loss,
 // adjustment should be based on logged/real data of loss stats/correlation.
@@ -71,8 +72,8 @@ struct VCMProtectionParameters
 
 
 /******************************/
-/* VCMProtectionMethod class    */
+/* VCMProtectionMethod class  */
-/****************************/
+/******************************/
 
 enum VCMProtectionMethodEnum
 {
@@ -250,14 +251,14 @@ public:
     //        - newMethodType    : New requested protection method type. If one
     //                           is already set, it will be deleted and replaced
     // Return value:             Returns true on update
-    bool SetMethod(enum VCMProtectionMethodEnum newMethodType);
+    bool SetMethod(VCMProtectionMethodEnum newMethodType);
 
     // Remove requested protection method
     // Input:
     //        - method          : method to be removed (if currently selected)
     //
     // Return value:             Returns true on update
-    bool RemoveMethod(enum VCMProtectionMethodEnum method);
+    bool RemoveMethod(VCMProtectionMethodEnum method);
 
     // Return required bit rate per selected protectin method
     float RequiredBitRate() const;
@@ -389,6 +390,7 @@ private:
     int                       _numLayers;
 };
 
+} // namespace media_optimization
 } // namespace webrtc
 
 #endif // WEBRTC_MODULES_VIDEO_CODING_MEDIA_OPT_UTIL_H_

webrtc/modules/video_coding/main/source/media_optimization.cc

@@ -16,6 +16,7 @@
 #include "webrtc/system_wrappers/interface/clock.h"
 
 namespace webrtc {
+namespace media_optimization {
 
 VCMMediaOptimization::VCMMediaOptimization(WebRtc_Word32 id,
                                            Clock* clock):
@@ -671,4 +672,5 @@ VCMMediaOptimization::InputFrameRate()
     return WebRtc_UWord32 (_incomingFrameRate + 0.5f);
 }
 
-}
+}  // namespace media_optimization
+}  // namespace webrtc

webrtc/modules/video_coding/main/source/media_optimization.h

@@ -17,16 +17,17 @@
 #include "media_opt_util.h"
 #include "qm_select.h"
 
-namespace webrtc
+namespace webrtc {
-{
-
-enum { kBitrateMaxFrameSamples = 60 };
-enum { kBitrateAverageWinMs    = 1000 };
 
 class Clock;
 class FrameDropper;
 class VCMContentMetricsProcessing;
 
+namespace media_optimization {
+
+enum { kBitrateMaxFrameSamples = 60 };
+enum { kBitrateAverageWinMs    = 1000 };
+
 struct VCMEncodedFrameSample
 {
     VCMEncodedFrameSample() : _sizeBytes(-1), _timeCompleteMs(-1) {}
@@ -204,6 +205,7 @@ private:
 
 }; // end of VCMMediaOptimization class definition
 
-} // namespace webrtc
+}  // namespace media_optimization
+}  // namespace webrtc
 
 #endif // WEBRTC_MODULES_VIDEO_CODING_MEDIA_OPTIMIZATION_H_

webrtc/modules/video_coding/main/source/receiver.cc

@@ -63,7 +63,7 @@ int32_t VCMReceiver::Initialize() {
   CriticalSectionScoped cs(crit_sect_);
   Reset();
   if (!master_) {
-    SetNackMode(kNoNack);
+    SetNackMode(kNoNack, -1, -1);
   }
   return VCM_OK;
 }
@@ -72,7 +72,8 @@ void VCMReceiver::UpdateRtt(uint32_t rtt) {
   jitter_buffer_.UpdateRtt(rtt);
 }
 
-int32_t VCMReceiver::InsertPacket(const VCMPacket& packet, uint16_t frame_width,
+int32_t VCMReceiver::InsertPacket(const VCMPacket& packet,
+                                  uint16_t frame_width,
                                   uint16_t frame_height) {
   // Find an empty frame.
   VCMEncodedFrame* buffer = NULL;
@@ -243,7 +244,7 @@ VCMEncodedFrame* VCMReceiver::FrameForDecoding(
     // No time to wait for a complete frame, check if we have an incomplete.
     const bool dual_receiver_enabled_and_passive = (dual_receiver != NULL &&
         dual_receiver->State() == kPassive &&
-        dual_receiver->NackMode() == kNackInfinite);
+        dual_receiver->NackMode() == kNack);
     if (dual_receiver_enabled_and_passive &&
         !jitter_buffer_.CompleteSequenceWithNextFrame()) {
       // Jitter buffer state might get corrupt with this frame.
@@ -269,7 +270,7 @@ VCMEncodedFrame* VCMReceiver::FrameForDecoding(
     // No time left to wait, we must decode this frame now.
     const bool dual_receiver_enabled_and_passive = (dual_receiver != NULL &&
         dual_receiver->State() == kPassive &&
-        dual_receiver->NackMode() == kNackInfinite);
+        dual_receiver->NackMode() == kNack);
     if (dual_receiver_enabled_and_passive &&
         !jitter_buffer_.CompleteSequenceWithNextFrame()) {
       // Jitter buffer state might get corrupt with this frame.
@@ -306,7 +307,7 @@ VCMEncodedFrame* VCMReceiver::FrameForRendering(uint16_t max_wait_time_ms,
     // Get an incomplete frame.
     const bool dual_receiver_enabled_and_passive = (dual_receiver != NULL &&
         dual_receiver->State() == kPassive &&
-        dual_receiver->NackMode() == kNackInfinite);
+        dual_receiver->NackMode() == kNack);
     if (dual_receiver_enabled_and_passive &&
         !jitter_buffer_.CompleteSequenceWithNextFrame()) {
       // Jitter buffer state might get corrupt with this frame.
@@ -340,10 +341,13 @@ uint32_t VCMReceiver::DiscardedPackets() const {
   return jitter_buffer_.num_discarded_packets();
 }
 
-void VCMReceiver::SetNackMode(VCMNackMode nackMode) {
+void VCMReceiver::SetNackMode(VCMNackMode nackMode,
+                              int low_rtt_nack_threshold_ms,
+                              int high_rtt_nack_threshold_ms) {
   CriticalSectionScoped cs(crit_sect_);
   // Default to always having NACK enabled in hybrid mode.
-  jitter_buffer_.SetNackMode(nackMode, kLowRttNackMs, -1);
+  jitter_buffer_.SetNackMode(nackMode, low_rtt_nack_threshold_ms,
+                             high_rtt_nack_threshold_ms);
   if (!master_) {
     state_ = kPassive;  // The dual decoder defaults to passive.
   }
@@ -361,24 +365,21 @@ VCMNackMode VCMReceiver::NackMode() const {
 }
 
 VCMNackStatus VCMReceiver::NackList(uint16_t* nack_list,
-                                    uint16_t* size) {
+                                    uint16_t size,
-  bool extended = false;
+                                    uint16_t* nack_list_length) {
-  uint16_t nack_list_size = 0;
+  bool request_key_frame = false;
-  uint16_t* internal_nack_list = jitter_buffer_.CreateNackList(&nack_list_size,
+  uint16_t* internal_nack_list = jitter_buffer_.GetNackList(
-                                                               &extended);
+      nack_list_length, &request_key_frame);
-  if (internal_nack_list == NULL && nack_list_size == 0xffff) {
+  if (request_key_frame) {
     // This combination is used to trigger key frame requests.
-    *size = 0;
     return kNackKeyFrameRequest;
   }
-  if (nack_list_size > *size) {
+  if (*nack_list_length > size) {
-    *size = nack_list_size;
     return kNackNeedMoreMemory;
   }
-  if (internal_nack_list != NULL && nack_list_size > 0) {
+  if (internal_nack_list != NULL && *nack_list_length > 0) {
-    memcpy(nack_list, internal_nack_list, nack_list_size * sizeof(uint16_t));
+    memcpy(nack_list, internal_nack_list, *nack_list_length * sizeof(uint16_t));
   }
-  *size = nack_list_size;
   return kNackOk;
 }
 

webrtc/modules/video_coding/main/source/receiver.h

@@ -58,11 +58,14 @@ class VCMReceiver {
   uint32_t DiscardedPackets() const;
 
   // NACK.
-  void SetNackMode(VCMNackMode nackMode);
+  void SetNackMode(VCMNackMode nackMode,
+                   int low_rtt_nack_threshold_ms,
+                   int high_rtt_nack_threshold_ms);
   void SetNackSettings(size_t max_nack_list_size,
                        int max_packet_age_to_nack);
   VCMNackMode NackMode() const;
-  VCMNackStatus NackList(uint16_t* nackList, uint16_t* size);
+  VCMNackStatus NackList(uint16_t* nackList, uint16_t size,
+                         uint16_t* nack_list_length);
 
   // Dual decoder.
   bool DualDecoderCaughtUp(VCMEncodedFrame* dual_frame,

webrtc/modules/video_coding/main/source/session_info.cc

@@ -352,186 +352,6 @@ int VCMSessionInfo::MakeDecodable() {
   return return_length;
 }
 
-int VCMSessionInfo::BuildHardNackList(int* seq_num_list,
-                                      int seq_num_list_length,
-                                      int nack_seq_nums_index) {
-  assert(seq_num_list && seq_num_list_length > 0);
-
-  if (packets_.empty() && empty_seq_num_low_ == -1) {
-    return nack_seq_nums_index;
-  }
-
-  // Find end point (index of entry equals the sequence number of the first
-  // packet).
-  int low_seq_num = (packets_.empty()) ? empty_seq_num_low_:
-      packets_.front().seqNum;
-  for (; nack_seq_nums_index < seq_num_list_length; ++nack_seq_nums_index) {
-    if (seq_num_list[nack_seq_nums_index] == low_seq_num) {
-      seq_num_list[nack_seq_nums_index] = -1;
-      ++nack_seq_nums_index;
-      break;
-    }
-  }
-
-  if (!packets_.empty()) {
-    // Zero out between the first entry and the end point.
-    PacketIterator it = packets_.begin();
-    PacketIterator prev_it = it;
-    ++it;
-    while (it != packets_.end() && nack_seq_nums_index < seq_num_list_length) {
-      if (!InSequence(it, prev_it)) {
-        // Found a sequence number gap due to packet loss.
-        nack_seq_nums_index += PacketsMissing(it, prev_it);
-        session_nack_ = true;
-      }
-      seq_num_list[nack_seq_nums_index] = -1;
-      ++nack_seq_nums_index;
-      prev_it = it;
-      ++it;
-    }
-    if (!packets_.front().isFirstPacket)
-        session_nack_ = true;
-  }
-  nack_seq_nums_index = ClearOutEmptyPacketSequenceNumbers(seq_num_list,
-                                                           seq_num_list_length,
-                                                           nack_seq_nums_index);
-  return nack_seq_nums_index;
-}
-
-int VCMSessionInfo::BuildSoftNackList(int* seq_num_list,
-                                      int seq_num_list_length,
-                                      int nack_seq_nums_index,
-                                      int rtt_ms) {
-  assert(seq_num_list && seq_num_list_length > 0);
-
-  if (packets_.empty() && empty_seq_num_low_ == -1) {
-    return nack_seq_nums_index;
-  }
-
-  int low_seq_num = (packets_.empty()) ? empty_seq_num_low_:
-      packets_.front().seqNum;
-  // Find entrance point (nack_seq_nums_index of entry equals the sequence
-  // number of the first packet).
-  for (; nack_seq_nums_index < seq_num_list_length; ++nack_seq_nums_index) {
-    if (seq_num_list[nack_seq_nums_index] == low_seq_num) {
-      seq_num_list[nack_seq_nums_index] = -1;
-      break;
-    }
-  }
-
-  // TODO(mikhal): 1. Update score based on RTT value 2. Add partition data.
-  // Use the previous available.
-  bool base_available = false;
-  if ((nack_seq_nums_index > 0) && (seq_num_list[nack_seq_nums_index] == -1)) {
-    // Found first packet, for now let's go only one back.
-    if ((seq_num_list[nack_seq_nums_index - 1] == -1) ||
-        (seq_num_list[nack_seq_nums_index - 1] == -2)) {
-      // This is indeed the first packet, as previous packet was populated.
-      base_available = true;
-    }
-  }
-  bool allow_nack = ((packets_.size() > 0 && !packets_.front().isFirstPacket)
-    || !base_available);
-
-  // Zero out between first entry and end point.
-
-  int media_high_seq_num;
-  if (HaveLastPacket()) {
-    media_high_seq_num = packets_.back().seqNum;
-  } else {
-    // Estimation.
-    if (empty_seq_num_low_ >= 0) {
-      // Assuming empty packets have later sequence numbers than media packets.
-      media_high_seq_num = empty_seq_num_low_ - 1;
-    } else {
-      // Since this frame doesn't have the marker bit we can assume it should
-      // contain at least one more packet.
-      media_high_seq_num = static_cast<uint16_t>(packets_.back().seqNum + 1);
-    }
-  }
-
-  // Compute session/packet scores and thresholds:
-  // based on RTT and layer info (when available).
-  float nack_score_threshold = 0.25f;
-  float layer_score = TemporalId() > 0 ? 0.0f : 1.0f;
-  float rtt_score = 1.0f;
-  float score_multiplier = rtt_score * layer_score;
-  // Zero out between first entry and end point.
-  if (!packets_.empty()) {
-    PacketIterator it = packets_.begin();
-    PacketIterator prev_it = it;
-    ++nack_seq_nums_index;
-    ++it;
-    // TODO(holmer): Rewrite this in a way which better makes use of the list.
-    while (it != packets_.end() && nack_seq_nums_index < seq_num_list_length) {
-    // Only process media packet sequence numbers.
-      if (LatestSequenceNumber((*it).seqNum, media_high_seq_num, NULL) ==
-        (*it).seqNum && (*it).seqNum != media_high_seq_num)
-        break;
-      if (!InSequence(it, prev_it)) {
-        // Found a sequence number gap due to packet loss.
-        int num_lost = PacketsMissing(it, prev_it);
-        for (int i = 0 ; i < num_lost; ++i) {
-          // Compute score of the packet.
-          float score = 1.0f;
-          // Multiply internal score (packet) by score multiplier.
-          score *= score_multiplier;
-          if (score > nack_score_threshold) {
-            allow_nack = true;
-          } else {
-            seq_num_list[nack_seq_nums_index] = -1;
-          }
-          ++nack_seq_nums_index;
-        }
-      }
-      seq_num_list[nack_seq_nums_index] = -1;
-      ++nack_seq_nums_index;
-      prev_it = it;
-      ++it;
-    }
-  }
-
-  nack_seq_nums_index = ClearOutEmptyPacketSequenceNumbers(seq_num_list,
-                                                           seq_num_list_length,
-                                                           nack_seq_nums_index);
-
-  session_nack_ = allow_nack;
-  return nack_seq_nums_index;
-}
-
-int VCMSessionInfo::ClearOutEmptyPacketSequenceNumbers(
-    int* seq_num_list,
-    int seq_num_list_length,
-    int index) const {
-  // Empty packets follow the data packets, and therefore have a higher
-  // sequence number. We do not want to NACK empty packets.
-  if (empty_seq_num_low_ != -1 && empty_seq_num_high_ != -1) {
-    // First make sure that we are at least at the minimum value (if not we are
-    // missing last packet(s)).
-    while (index < seq_num_list_length &&
-           seq_num_list[index] < empty_seq_num_low_) {
-      ++index;
-    }
-
-    // Mark empty packets.
-    while (index < seq_num_list_length &&
-           seq_num_list[index] >= 0 &&
-           seq_num_list[index] <= empty_seq_num_high_) {
-      seq_num_list[index] = -2;
-      ++index;
-    }
-  }
-  return index;
-}
-
-int VCMSessionInfo::PacketsMissing(const PacketIterator& packet_it,
-                                   const PacketIterator& prev_packet_it) {
-  if (packet_it == prev_packet_it)
-    return 0;
-  return static_cast<uint16_t>((*packet_it).seqNum -
-                               (*prev_packet_it).seqNum - 1);
-}
-
 bool
 VCMSessionInfo::HaveLastPacket() const {
   return (!packets_.empty() && packets_.back().markerBit);

webrtc/modules/video_coding/main/source/session_info.h

@@ -101,8 +101,6 @@ class VCMSessionInfo {
   PacketIterator FindPartitionEnd(PacketIterator it) const;
   static bool InSequence(const PacketIterator& it,
                          const PacketIterator& prev_it);
-  static int PacketsMissing(const PacketIterator& packet_it,
-                            const PacketIterator& prev_packet_it);
   int InsertBuffer(uint8_t* frame_buffer,
                    PacketIterator packetIterator);
   void ShiftSubsequentPackets(PacketIterator it, int steps_to_shift);
@@ -117,14 +115,6 @@ class VCMSessionInfo {
   // would be sent to the decoder.
   void UpdateDecodableSession(int rtt_ms);
 
-  // Clears the sequence numbers in |seq_num_list| of any empty packets received
-  // in this session. |index| is an index in the list at which we start looking
-  // for the sequence numbers. When done this function returns the index of the
-  // next element in the list.
-  int ClearOutEmptyPacketSequenceNumbers(int* seq_num_list,
-                                         int seq_num_list_length,
-                                         int index) const;
-
   // If this session has been NACKed by the jitter buffer.
   bool session_nack_;
   bool complete_;

webrtc/modules/video_coding/main/source/session_info_unittest.cc

@@ -788,165 +788,4 @@ TEST_F(TestNalUnits, ReorderWrapLosses) {
   EXPECT_EQ(0, session_.SessionLength());
   EXPECT_EQ(2, session_.packets_not_decodable());
 }
-
-TEST_F(TestNackList, NoLosses) {
-  uint16_t low = 0xFFFF - 5;
-
-  packet_.seqNum = low;
-  packet_.isFirstPacket = true;
-  packet_.markerBit = false;
-  FillPacket(0);
-  ASSERT_EQ(session_.InsertPacket(packet_, frame_buffer_, false, 0),
-            kPacketBufferSize);
-
-  for (int i = 1; i < 9; ++i) {
-    packet_.seqNum += 1;
-    packet_.isFirstPacket = false;
-    packet_.markerBit = false;
-    FillPacket(i + 1);
-    ASSERT_EQ(session_.InsertPacket(packet_, frame_buffer_, false, 0),
-              kPacketBufferSize);
-  }
-
-  packet_.seqNum += 1;
-  packet_.isFirstPacket = false;
-  packet_.markerBit = true;
-  FillPacket(10);
-  ASSERT_EQ(session_.InsertPacket(packet_, frame_buffer_, false, 0),
-            kPacketBufferSize);
-
-  EXPECT_EQ(10 * kPacketBufferSize, session_.SessionLength());
-  BuildSeqNumList(low, packet_.seqNum);
-  EXPECT_EQ(seq_num_list_length_, session_.BuildHardNackList(
-      seq_num_list_, seq_num_list_length_, 0));
-  EXPECT_FALSE(session_.session_nack());
-  SCOPED_TRACE("Calling VerifyAll");
-  VerifyAll(-1);
-
-  BuildSeqNumList(low, packet_.seqNum);
-  EXPECT_EQ(seq_num_list_length_, session_.BuildSoftNackList(
-      seq_num_list_, seq_num_list_length_, 0, 60));
-  SCOPED_TRACE("Calling VerifyAll");
-  VerifyAll(-1);
-}
-
-TEST_F(TestNackList, FiveLossesSpreadOut) {
-  uint16_t low = 0xFFFF - 5;
-
-  packet_.seqNum = low;
-  packet_.isFirstPacket = false;
-  packet_.markerBit = true;
-  FillPacket(0);
-  ASSERT_EQ(session_.InsertPacket(packet_, frame_buffer_, false, 0),
-            kPacketBufferSize);
-
-  for (int i = 1; i < 9; ++i) {
-    packet_.seqNum += 1;
-    packet_.isFirstPacket = false;
-    packet_.markerBit = false;
-    FillPacket(i);
-    if ((i + 1) % 2)
-      ASSERT_EQ(session_.InsertPacket(packet_, frame_buffer_, false, 0),
-                kPacketBufferSize);
-  }
-
-  packet_.seqNum++;  // Simulate loss of last packet.
-
-  EXPECT_EQ(5 * kPacketBufferSize, session_.SessionLength());
-  BuildSeqNumList(low, packet_.seqNum);
-  // Will be at |seq_num_list_length - 1| since the last packet is missing.
-  EXPECT_EQ(seq_num_list_length_ - 1, session_.BuildHardNackList(
-      seq_num_list_, seq_num_list_length_, 0));
-  for (int i = 0; i < seq_num_list_length_; ++i) {
-    if (i % 2)
-      EXPECT_EQ(static_cast<uint16_t>(low + i), seq_num_list_[i]);
-    else
-      EXPECT_EQ(-1, seq_num_list_[i]);
-  }
-
-  BuildSeqNumList(low, packet_.seqNum);
-  // Will be at |seq_num_list_length - 1| since the last packet is missing.
-  EXPECT_EQ(seq_num_list_length_ - 1, session_.BuildSoftNackList(
-      seq_num_list_, seq_num_list_length_, 0, 60));
-  EXPECT_EQ(true, session_.session_nack());
-  for (int i = 0; i < seq_num_list_length_; ++i) {
-    if (i % 2)
-      EXPECT_EQ(static_cast<uint16_t>(low + i), seq_num_list_[i]);
-    else
-      EXPECT_EQ(-1, seq_num_list_[i]);
-  }
-}
-
-TEST_F(TestNackList, FirstAndLastLost) {
-  uint16_t low = 0xFFFF;
-
-  packet_.seqNum = low + 1;
-  packet_.isFirstPacket = false;
-  packet_.markerBit = false;
-  FillPacket(0);
-  ASSERT_EQ(session_.InsertPacket(packet_, frame_buffer_, false, 0),
-            kPacketBufferSize);
-
-  EXPECT_EQ(kPacketBufferSize, session_.SessionLength());
-  BuildSeqNumList(low, packet_.seqNum + 1);
-  // Will be at |seq_num_list_length - 1| since the last packet is missing.
-  EXPECT_EQ(seq_num_list_length_ - 1, session_.BuildHardNackList(
-      seq_num_list_, seq_num_list_length_, 0));
-  EXPECT_EQ(0xFFFF, seq_num_list_[0]);
-  EXPECT_EQ(-1, seq_num_list_[1]);
-  EXPECT_EQ(1, seq_num_list_[2]);
-
-  BuildSeqNumList(low, packet_.seqNum + 1);
-  // Will be at |seq_num_list_length - 1| since the last packet is missing.
-  EXPECT_EQ(seq_num_list_length_ - 1, session_.BuildSoftNackList(
-      seq_num_list_, seq_num_list_length_, 0, 60));
-  EXPECT_EQ(true, session_.session_nack());
-  EXPECT_EQ(0xFFFF, seq_num_list_[0]);
-  EXPECT_EQ(-1, seq_num_list_[1]);
-  EXPECT_EQ(1, seq_num_list_[2]);
-}
-
-TEST_F(TestNackList, LostAllButEmptyPackets) {
-  uint16_t low = 0;
-  packet_.seqNum = low + 1;
-  packet_.isFirstPacket = false;
-  packet_.markerBit = false;
-  packet_.frameType = kFrameEmpty;
-  packet_.sizeBytes = 0;
-  FillPacket(0);
-  ASSERT_EQ(session_.InsertPacket(packet_, frame_buffer_, false, 0), 0);
-
-  packet_.seqNum = low + 3;
-  packet_.isFirstPacket = false;
-  packet_.markerBit = false;
-  packet_.frameType = kFrameEmpty;
-  packet_.sizeBytes = 0;
-  FillPacket(0);
-  ASSERT_EQ(session_.InsertPacket(packet_, frame_buffer_, false, 0), 0);
-
-  // Test soft NACKing.
-  EXPECT_EQ(0, session_.SessionLength());
-  BuildSeqNumList(low, packet_.seqNum + 1);
-  // Will be at |seq_num_list_length - 1| since the last packet is missing.
-  EXPECT_EQ(seq_num_list_length_ - 1, session_.BuildSoftNackList(
-      seq_num_list_, seq_num_list_length_, 0, 60));
-  EXPECT_EQ(true, session_.session_nack());
-  EXPECT_EQ(0, seq_num_list_[0]);
-  EXPECT_EQ(-1, seq_num_list_[1]);
-  EXPECT_EQ(-2, seq_num_list_[2]);
-  EXPECT_EQ(-2, seq_num_list_[3]);
-  EXPECT_EQ(4, seq_num_list_[4]);
-
-  // Test hard NACKing.
-  BuildSeqNumList(low, packet_.seqNum + 1);
-  // Will be at |seq_num_list_length - 1| since the last packet is missing.
-  EXPECT_EQ(seq_num_list_length_ - 1, session_.BuildHardNackList(
-      seq_num_list_, seq_num_list_length_, 0));
-  EXPECT_EQ(true, session_.session_nack());
-  EXPECT_EQ(0, seq_num_list_[0]);
-  EXPECT_EQ(-1, seq_num_list_[1]);
-  EXPECT_EQ(-2, seq_num_list_[2]);
-  EXPECT_EQ(-2, seq_num_list_[3]);
-  EXPECT_EQ(4, seq_num_list_[4]);
-}
 }  // namespace webrtc

webrtc/modules/video_coding/main/source/video_coding_impl.cc

@@ -538,7 +538,7 @@ VideoCodingModuleImpl::SetVideoProtection(VCMVideoProtection videoProtection,
     case kProtectionNackSender:
         {
             CriticalSectionScoped cs(_sendCritSect);
-            _mediaOpt.EnableProtectionMethod(enable, kNack);
+            _mediaOpt.EnableProtectionMethod(enable, media_optimization::kNack);
             break;
         }
 
@@ -547,11 +547,12 @@ VideoCodingModuleImpl::SetVideoProtection(VCMVideoProtection videoProtection,
             CriticalSectionScoped cs(_receiveCritSect);
             if (enable)
             {
-                _receiver.SetNackMode(kNackInfinite);
+              // Enable NACK and always wait for retransmits.
+                _receiver.SetNackMode(kNack, -1, -1);
             }
             else
             {
-                _receiver.SetNackMode(kNoNack);
+                _receiver.SetNackMode(kNoNack, -1, -1);
             }
             break;
         }
@@ -561,12 +562,16 @@ VideoCodingModuleImpl::SetVideoProtection(VCMVideoProtection videoProtection,
             CriticalSectionScoped cs(_receiveCritSect);
             if (enable)
             {
-                _receiver.SetNackMode(kNoNack);
+                // Enable NACK but don't wait for retransmissions and don't
-                _dualReceiver.SetNackMode(kNackInfinite);
+                // add any extra delay.
+                _receiver.SetNackMode(kNack, 0, 0);
+                // Enable NACK and always wait for retransmissions and
+                // compensate with extra delay.
+                _dualReceiver.SetNackMode(kNack, -1, -1);
             }
             else
             {
-                _dualReceiver.SetNackMode(kNoNack);
+                _dualReceiver.SetNackMode(kNoNack, -1, -1);
             }
             break;
         }
@@ -614,17 +619,23 @@ VideoCodingModuleImpl::SetVideoProtection(VCMVideoProtection videoProtection,
                 CriticalSectionScoped cs(_receiveCritSect);
                 if (enable)
                 {
-                    _receiver.SetNackMode(kNackHybrid);
+                    // Enable hybrid NACK/FEC. Always wait for retransmissions
+                    // and don't add extra delay when RTT is above
+                    // kLowRttNackMs.
+                    _receiver.SetNackMode(kNackHybrid,
+                                          media_optimization::kLowRttNackMs,
+                                          -1);
                 }
                 else
                 {
-                    _receiver.SetNackMode(kNoNack);
+                    _receiver.SetNackMode(kNoNack, -1, -1);
                 }
             }
             // Send Side
             {
                 CriticalSectionScoped cs(_sendCritSect);
-                _mediaOpt.EnableProtectionMethod(enable, kNackFec);
+                _mediaOpt.EnableProtectionMethod(enable,
+                                                 media_optimization::kNackFec);
             }
             break;
         }
@@ -632,7 +643,7 @@ VideoCodingModuleImpl::SetVideoProtection(VCMVideoProtection videoProtection,
     case kProtectionFEC:
         {
             CriticalSectionScoped cs(_sendCritSect);
-            _mediaOpt.EnableProtectionMethod(enable, kFec);
+            _mediaOpt.EnableProtectionMethod(enable, media_optimization::kFec);
             break;
         }
 
@@ -853,7 +864,7 @@ VideoCodingModuleImpl::Decode(WebRtc_UWord16 maxWaitTimeMs)
 
     const bool dualReceiverEnabledNotReceiving =
         (_dualReceiver.State() != kReceiving &&
-         _dualReceiver.NackMode() == kNackInfinite);
+         _dualReceiver.NackMode() == kNack);
 
     VCMEncodedFrame* frame = _receiver.FrameForDecoding(
         maxWaitTimeMs,
@@ -984,7 +995,7 @@ VideoCodingModuleImpl::DecodeDualFrame(WebRtc_UWord16 maxWaitTimeMs)
 {
     CriticalSectionScoped cs(_receiveCritSect);
     if (_dualReceiver.State() != kReceiving ||
-        _dualReceiver.NackMode() != kNackInfinite)
+        _dualReceiver.NackMode() != kNack)
     {
         // The dual receiver is currently not receiving or
         // dual decoder mode is disabled.
@@ -1203,8 +1214,11 @@ VideoCodingModuleImpl::IncomingPacket(const WebRtc_UWord8* incomingPayload,
           return ret;
         }
     }
-    ret = _receiver.InsertPacket(packet, rtpInfo.type.Video.width,
+    ret = _receiver.InsertPacket(packet,
+                                 rtpInfo.type.Video.width,
                                  rtpInfo.type.Video.height);
+    // TODO(holmer): Investigate if this somehow should use the key frame
+    // request scheduling to throttle the requests.
     if (ret == VCM_FLUSH_INDICATOR) {
       RequestKeyFrame();
       ResetDecoder();
@@ -1245,21 +1259,19 @@ WebRtc_Word32
 VideoCodingModuleImpl::NackList(WebRtc_UWord16* nackList, WebRtc_UWord16& size)
 {
     VCMNackStatus nackStatus = kNackOk;
+    uint16_t nack_list_length = 0;
     // Collect sequence numbers from the default receiver
     // if in normal nack mode. Otherwise collect them from
     // the dual receiver if the dual receiver is receiving.
     if (_receiver.NackMode() != kNoNack)
     {
-        nackStatus = _receiver.NackList(nackList, &size);
+        nackStatus = _receiver.NackList(nackList, size, &nack_list_length);
     }
-    else if (_dualReceiver.State() != kPassive)
+    if (nack_list_length == 0 && _dualReceiver.State() != kPassive)
     {
-        nackStatus = _dualReceiver.NackList(nackList, &size);
+        nackStatus = _dualReceiver.NackList(nackList, size, &nack_list_length);
-    }
-    else
-    {
-        size = 0;
     }
+    size = nack_list_length;
 
     switch (nackStatus)
     {
@@ -1302,10 +1314,10 @@ int VideoCodingModuleImpl::SetSenderNackMode(SenderNackMode mode) {
 
   switch (mode) {
     case kNackNone:
-      _mediaOpt.EnableProtectionMethod(false, kNack);
+      _mediaOpt.EnableProtectionMethod(false, media_optimization::kNack);
       break;
     case kNackAll:
-      _mediaOpt.EnableProtectionMethod(true, kNack);
+      _mediaOpt.EnableProtectionMethod(true, media_optimization::kNack);
       break;
     case kNackSelective:
       return VCM_NOT_IMPLEMENTED;
@@ -1320,7 +1332,7 @@ int VideoCodingModuleImpl::SetSenderReferenceSelection(bool enable) {
 
 int VideoCodingModuleImpl::SetSenderFEC(bool enable) {
   CriticalSectionScoped cs(_sendCritSect);
-  _mediaOpt.EnableProtectionMethod(enable, kFec);
+  _mediaOpt.EnableProtectionMethod(enable, media_optimization::kFec);
   return VCM_OK;
 }
 
@@ -1334,8 +1346,8 @@ int VideoCodingModuleImpl::SetReceiverRobustnessMode(
   CriticalSectionScoped cs(_receiveCritSect);
   switch (robustnessMode) {
     case kNone:
-      _receiver.SetNackMode(kNoNack);
+      _receiver.SetNackMode(kNoNack, -1, -1);
-      _dualReceiver.SetNackMode(kNoNack);
+      _dualReceiver.SetNackMode(kNoNack, -1, -1);
       if (errorMode == kNoDecodeErrors) {
         _keyRequestMode = kKeyOnLoss;
       } else {
@@ -1346,23 +1358,29 @@ int VideoCodingModuleImpl::SetReceiverRobustnessMode(
       if (errorMode == kAllowDecodeErrors) {
         return VCM_PARAMETER_ERROR;
       }
-      _receiver.SetNackMode(kNackInfinite);
+      // Always wait for retransmissions.
-      _dualReceiver.SetNackMode(kNoNack);
+      _receiver.SetNackMode(kNack, -1, -1);
+      _dualReceiver.SetNackMode(kNoNack, -1, -1);
       _keyRequestMode = kKeyOnError;  // TODO(hlundin): On long NACK list?
       break;
     case kSoftNack:
       assert(false); // TODO(hlundin): Not completed.
       return VCM_NOT_IMPLEMENTED;
-      _receiver.SetNackMode(kNackHybrid);
+      // Enable hybrid NACK/FEC. Always wait for retransmissions and don't add
-      _dualReceiver.SetNackMode(kNoNack);
+      // extra delay when RTT is above kLowRttNackMs.
+      _receiver.SetNackMode(kNackHybrid, media_optimization::kLowRttNackMs, -1);
+      _dualReceiver.SetNackMode(kNoNack, -1, -1);
       _keyRequestMode = kKeyOnError;
       break;
     case kDualDecoder:
       if (errorMode == kNoDecodeErrors) {
         return VCM_PARAMETER_ERROR;
       }
-      _receiver.SetNackMode(kNoNack);
+      // Enable NACK but don't wait for retransmissions and don't add any extra
-      _dualReceiver.SetNackMode(kNackInfinite);
+      // delay.
+      _receiver.SetNackMode(kNack, 0, 0);
+      // Enable NACK, compensate with extra delay and wait for retransmissions.
+      _dualReceiver.SetNackMode(kNack, -1, -1);
       _keyRequestMode = kKeyOnError;
       break;
     case kReferenceSelection:
@@ -1371,8 +1389,8 @@ int VideoCodingModuleImpl::SetReceiverRobustnessMode(
       if (errorMode == kNoDecodeErrors) {
         return VCM_PARAMETER_ERROR;
       }
-      _receiver.SetNackMode(kNoNack);
+      _receiver.SetNackMode(kNoNack, -1, -1);
-      _dualReceiver.SetNackMode(kNoNack);
+      _dualReceiver.SetNackMode(kNoNack, -1, -1);
       break;
   }
   return VCM_OK;

webrtc/modules/video_coding/main/source/video_coding_impl.h

@@ -307,7 +307,7 @@ private:
     VCMGenericEncoder*                  _encoder;
     VCMEncodedFrameCallback             _encodedFrameCallback;
     std::vector<FrameType>              _nextFrameTypes;
-    VCMMediaOptimization                _mediaOpt;
+    media_optimization::VCMMediaOptimization _mediaOpt;
     VideoCodecType                      _sendCodecType;
     VCMSendStatisticsCallback*          _sendStatsCallback;
     FILE*                               _encoderInputFile;

webrtc/modules/video_coding/main/source/video_coding_impl_unittest.cc

@@ -13,8 +13,8 @@
 #include "webrtc/modules/video_coding/codecs/interface/mock/mock_video_codec_interface.h"
 #include "webrtc/modules/video_coding/main/interface/mock/mock_vcm_callbacks.h"
 #include "webrtc/modules/video_coding/main/interface/video_coding.h"
+#include "webrtc/system_wrappers/interface/clock.h"
 #include "webrtc/system_wrappers/interface/scoped_ptr.h"
-#include "webrtc/system_wrappers/interface/tick_util.h"
 
 #include "gtest/gtest.h"
 
@@ -38,8 +38,8 @@ class TestVideoCodingModule : public ::testing::Test {
   static const int kUnusedPayloadType = 10;
 
   virtual void SetUp() {
-    TickTime::UseFakeClock(0);
+    clock_.reset(new SimulatedClock(0));
-    vcm_ = VideoCodingModule::Create(0);
+    vcm_ = VideoCodingModule::Create(0, clock_.get());
     EXPECT_EQ(0, vcm_->InitializeReceiver());
     EXPECT_EQ(0, vcm_->InitializeSender());
     EXPECT_EQ(0, vcm_->RegisterExternalEncoder(&encoder_, kUnusedPayloadType,
@@ -104,8 +104,6 @@ class TestVideoCodingModule : public ::testing::Test {
       EXPECT_EQ(0, vcm_->IncomingPacket(payload, 0, *header));
       ++header->header.sequenceNumber;
     }
-    EXPECT_CALL(packet_request_callback_, ResendPackets(_, _))
-        .Times(0);
     EXPECT_EQ(0, vcm_->Process());
     EXPECT_CALL(decoder_, Decode(_, _, _, _, _))
         .Times(0);
@@ -126,6 +124,7 @@ class TestVideoCodingModule : public ::testing::Test {
   }
 
   VideoCodingModule* vcm_;
+  scoped_ptr<SimulatedClock> clock_;
   NiceMock<MockVideoDecoder> decoder_;
   NiceMock<MockVideoEncoder> encoder_;
   I420VideoFrame input_frame_;
@@ -194,8 +193,10 @@ TEST_F(TestVideoCodingModule, PaddingOnlyFrames) {
   header.header.headerLength = 12;
   header.type.Video.codec = kRTPVideoVP8;
   for (int i = 0; i < 10; ++i) {
+    EXPECT_CALL(packet_request_callback_, ResendPackets(_, _))
+        .Times(0);
     InsertAndVerifyPaddingFrame(payload, 0, &header);
-    TickTime::AdvanceFakeClock(33);
+    clock_->AdvanceTimeMilliseconds(33);
     header.header.timestamp += 3000;
   }
 }
@@ -220,7 +221,7 @@ TEST_F(TestVideoCodingModule, PaddingOnlyFramesWithLosses) {
   header.type.Video.isFirstPacket = true;
   header.header.markerBit = true;
   InsertAndVerifyDecodableFrame(payload, kFrameSize, &header);
-  TickTime::AdvanceFakeClock(33);
+  clock_->AdvanceTimeMilliseconds(33);
   header.header.timestamp += 3000;
 
   header.frameType = kFrameEmpty;
@@ -228,17 +229,27 @@ TEST_F(TestVideoCodingModule, PaddingOnlyFramesWithLosses) {
   header.header.markerBit = false;
   // Insert padding frames.
   for (int i = 0; i < 10; ++i) {
-    // Lose the 4th frame.
-    if (i == 3) {
-      header.header.sequenceNumber += 5;
-      ++i;
-    }
     // Lose one packet from the 6th frame.
     if (i == 5) {
       ++header.header.sequenceNumber;
     }
-    InsertAndVerifyPaddingFrame(payload, 0, &header);
+    // Lose the 4th frame.
-    TickTime::AdvanceFakeClock(33);
+    if (i == 3) {
+      header.header.sequenceNumber += 5;
+    } else {
+      if (i > 3 && i < 5) {
+        EXPECT_CALL(packet_request_callback_, ResendPackets(_, 5))
+            .Times(1);
+      } else if (i >= 5) {
+        EXPECT_CALL(packet_request_callback_, ResendPackets(_, 6))
+            .Times(1);
+      } else {
+        EXPECT_CALL(packet_request_callback_, ResendPackets(_, _))
+            .Times(0);
+      }
+      InsertAndVerifyPaddingFrame(payload, 0, &header);
+    }
+    clock_->AdvanceTimeMilliseconds(33);
     header.header.timestamp += 3000;
   }
 }
@@ -271,7 +282,7 @@ TEST_F(TestVideoCodingModule, PaddingOnlyAndVideo) {
       header.type.Video.isFirstPacket = true;
       header.header.markerBit = true;
       InsertAndVerifyDecodableFrame(payload, kFrameSize, &header);
-      TickTime::AdvanceFakeClock(33);
+      clock_->AdvanceTimeMilliseconds(33);
       header.header.timestamp += 3000;
     }
 
@@ -281,7 +292,7 @@ TEST_F(TestVideoCodingModule, PaddingOnlyAndVideo) {
     header.header.markerBit = false;
     for (int j = 0; j < 2; ++j) {
       InsertAndVerifyPaddingFrame(payload, 0, &header);
-      TickTime::AdvanceFakeClock(33);
+      clock_->AdvanceTimeMilliseconds(33);
       header.header.timestamp += 3000;
     }
   }