Add support for handling reordered SS data on the receive-side for VP9.

webrtc/modules/video_coding/main/source/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/main/source/jitter_buffer.h"
@@ skipping 12 matching lines @@
 #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/event_wrapper.h"
 #include "webrtc/system_wrappers/interface/logging.h"
 #include "webrtc/system_wrappers/interface/metrics.h"
 #include "webrtc/system_wrappers/interface/trace_event.h"
 
 namespace webrtc {
 
+// Interval for updating SS data.
+static const uint32_t kPeriodicUpdateIntervalSec = 60;
+
 // Use this rtt if no value has been reported.
 static const int64_t kDefaultRtt = 200;
 
 typedef std::pair<uint32_t, VCMFrameBuffer*> FrameListPair;
 
 bool IsKeyFrame(FrameListPair pair) {
   return pair.second->FrameType() == kVideoFrameKey;
 }
 
 bool HasNonEmptyState(FrameListPair pair) {
@@ skipping 63 matching lines @@
 }
 
 void FrameList::Reset(UnorderedFrameList* free_frames) {
   while (!empty()) {
     begin()->second->Reset();
     free_frames->push_back(begin()->second);
     erase(begin());
   }
 }
 
+bool Vp9SsMap::Insert(const VCMPacket& packet) {
+  if (!packet.codecSpecificHeader.codecHeader.VP9.ss_data_available)
+    return false;
+
+  ss_map_[packet.timestamp] = packet.codecSpecificHeader.codecHeader.VP9.gof;
+  return true;
+}
+
+void Vp9SsMap::Reset() {
+  ss_map_.clear();
+}
+
+bool Vp9SsMap::Find(uint32_t timestamp, uint32_t* ss_timestamp) const {
+  bool found = false;
+  for (const auto& it : ss_map_) {
+    if (it.first == timestamp || IsNewerTimestamp(timestamp, it.first)) {
+      *ss_timestamp = it.first;
+      found = true;
+    }
+  }
+  return found;
+}

[stefan-webrtc, 2015/10/13 13:49:43]

You may be able to implement this with ss_map_.lower_bound(timestamp) in some
way, not sure if it would be worth it though. Probably not since you will only
have one or two SS here, right?

[åsapersson, 2015/10/14 15:13:51]

Should be around one or two (or as many that are received during a minute (old
currently cleaned up each minute)).

[stefan-webrtc, 2015/10/16 06:45:05]

Acknowledged.

+
+void Vp9SsMap::RemoveOld(uint32_t timestamp) {
+  if (!TimeForPeriodicUpdate(timestamp))
+    return;
+
+  uint32_t ss_timestamp;
+  if (!Find(timestamp, &ss_timestamp))

[stefan-webrtc, 2015/10/13 13:49:43]

It would be nice if Find() instead could return an iterator so that you can
simply use that to do an erase.

ss_map_.erase(ss_map_.begin(), it);

[åsapersson, 2015/10/14 15:13:50]

Done.

+    return;
+
+  RemoveOlderThan(ss_timestamp);
+  AdvanceFront(timestamp);
+}
+
+bool Vp9SsMap::TimeForPeriodicUpdate(uint32_t timestamp) const {

[stefan-webrtc, 2015/10/13 13:49:43]

What exactly is a periodic update?

[åsapersson, 2015/10/14 15:13:51]

Renamed this function. Looks for if it is time to do a clean up. Thought it
should not be done too often? This is also advancing the oldest SS data to
handle timestamp wrap in case the SS data are received very seldom (e.g. only
once in beginning, second when IsNewerTimestamp not true).

[stefan-webrtc, 2015/10/16 06:45:05]

Please add this as a comment to the method in the .h file.

[åsapersson, 2015/10/16 10:52:26]

Done.

+  if (ss_map_.empty() || !IsNewerTimestamp(timestamp, ss_map_.begin()->first))
+    return false;
+
+  uint32_t diff_sec = (timestamp - ss_map_.begin()->first) / 90000U;

[stefan-webrtc, 2015/10/13 13:49:43]

Name the constant 90000. We should already have that constant somewhere, right?

[åsapersson, 2015/10/14 15:13:50]

Done.

+  return diff_sec >= kPeriodicUpdateIntervalSec;
+}
+
+void Vp9SsMap::RemoveOlderThan(uint32_t timestamp) {
+  while (!ss_map_.empty() &&
+      IsNewerTimestamp(timestamp, ss_map_.begin()->first)) {
+    ss_map_.erase(ss_map_.begin());
+  }
+}
+
+void Vp9SsMap::AdvanceFront(uint32_t timestamp) {
+  assert(!ss_map_.empty());
+  GofInfoVP9 gof = ss_map_.begin()->second;
+  ss_map_.erase(ss_map_.begin());
+  ss_map_[timestamp] = gof;

[stefan-webrtc, 2015/10/13 13:49:43]

How is this supposed to work? Looks like we are changing the timestamp of the
oldest gof? Why?

[åsapersson, 2015/10/14 15:13:51]

See comment above.

+}
+
+bool Vp9SsMap::UpdatePacket(const VCMPacket* packet)  {

[stefan-webrtc, 2015/10/13 13:49:43]

Can this method be const?

[åsapersson, 2015/10/14 15:13:50]

Done.

+  uint8_t gof_idx = packet->codecSpecificHeader.codecHeader.VP9.gof_idx;
+  if (gof_idx == kNoGofIdx)
+    return false;  // No update needed.
+
+  uint32_t ss_timestamp;
+  if (!Find(packet->timestamp, &ss_timestamp))
+    return false;  // Corresponding SS not yet received.
+
+  auto it = ss_map_.find(ss_timestamp);
+  if (gof_idx >= it->second.num_frames_in_gof)
+    return false;  // Corresponding SS not yet received.
+
+  RTPVideoTypeHeader* hdr = const_cast<RTPVideoTypeHeader*>(

[stefan-webrtc, 2015/10/13 13:49:43]

Preferably no const_casts... Can we do it in some other way? At least do the
const_cast when calling the method.

[åsapersson, 2015/10/14 15:13:50]

Moved const_cast to when calling function. 

+      &packet->codecSpecificHeader.codecHeader);
+  hdr->VP9.temporal_idx = it->second.temporal_idx[gof_idx];
+  hdr->VP9.temporal_up_switch = it->second.temporal_up_switch[gof_idx];
+
+  // TODO(asapersson): Set hdr->VP9.ref_picture_id[i] and add usage.
+  hdr->VP9.num_ref_pics = it->second.num_ref_pics[gof_idx];
+  for (size_t i = 0; i < it->second.num_ref_pics[gof_idx]; ++i) {
+    hdr->VP9.pid_diff[i] = it->second.pid_diff[gof_idx][i];
+  }
+  return true;
+}
+
+void Vp9SsMap::UpdateFrames(FrameList* frames) {
+  for (FrameList::iterator it = frames->begin(); it != frames->end(); ++it) {

[stefan-webrtc, 2015/10/13 13:49:43]

for (auto& it : *frames)

[åsapersson, 2015/10/14 15:13:51]

Done.

+    uint8_t gof_idx = it->second->CodecSpecific()->codecSpecific.VP9.gof_idx;
+    if (gof_idx == kNoGofIdx) {
+      continue;
+    }
+    uint32_t ss_timestamp;
+    if (Find(it->second->TimeStamp(), &ss_timestamp)) {
+      auto ss_iter = ss_map_.find(ss_timestamp);

[stefan-webrtc, 2015/10/13 13:49:42]

Wouldn't it be better to have Find() return this iterator?

[åsapersson, 2015/10/14 15:13:51]

Done.

+      if (gof_idx >= ss_iter->second.num_frames_in_gof) {
+        continue;

[stefan-webrtc, 2015/10/13 13:49:43]

Does this mean we haven't received the right SS yet? Or should we log a warning?

[åsapersson, 2015/10/14 15:13:51]

The right not yet received, added a comment.

+      }
+      it->second->SetGofInfo(ss_iter->second, gof_idx);
+    }
+  }
+}
+
 VCMJitterBuffer::VCMJitterBuffer(Clock* clock,
                                  rtc::scoped_ptr<EventWrapper> event)
     : clock_(clock),
       running_(false),
       crit_sect_(CriticalSectionWrapper::CreateCriticalSection()),
       frame_event_(event.Pass()),
       max_number_of_frames_(kStartNumberOfFrames),
       free_frames_(),
       decodable_frames_(),
       incomplete_frames_(),
       last_decoded_state_(),
       first_packet_since_reset_(true),
-      last_gof_timestamp_(0),
-      last_gof_valid_(false),
       stats_callback_(NULL),
       incoming_frame_rate_(0),
       incoming_frame_count_(0),
       time_last_incoming_frame_count_(0),
       incoming_bit_count_(0),
       incoming_bit_rate_(0),
       num_consecutive_old_packets_(0),
       num_packets_(0),
       num_duplicated_packets_(0),
       num_discarded_packets_(0),
@@ skipping 75 matching lines @@
   num_discarded_packets_ = 0;
   time_first_packet_ms_ = 0;
 
   // Start in a non-signaled state.
   waiting_for_completion_.frame_size = 0;
   waiting_for_completion_.timestamp = 0;
   waiting_for_completion_.latest_packet_time = -1;
   first_packet_since_reset_ = true;
   rtt_ms_ = kDefaultRtt;
   last_decoded_state_.Reset();
-  last_gof_valid_ = false;
+  vp9_ss_map_.Reset();
 }
 
 void VCMJitterBuffer::Stop() {
   crit_sect_->Enter();
   UpdateHistograms();
   running_ = false;
   last_decoded_state_.Reset();
-  last_gof_valid_ = false;
+  vp9_ss_map_.Reset();
 
   // 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);
   }
@@ skipping 11 matching lines @@
 bool VCMJitterBuffer::Running() const {
   CriticalSectionScoped cs(crit_sect_);
   return running_;
 }
 
 void VCMJitterBuffer::Flush() {
   CriticalSectionScoped cs(crit_sect_);
   decodable_frames_.Reset(&free_frames_);
   incomplete_frames_.Reset(&free_frames_);
   last_decoded_state_.Reset();  // TODO(mikhal): sync reset.
-  last_gof_valid_ = false;
+  vp9_ss_map_.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;
   waiting_for_completion_.latest_packet_time = -1;
   first_packet_since_reset_ = true;
   missing_sequence_numbers_.clear();
 }
@@ skipping 310 matching lines @@
       LOG(LS_WARNING)
           << num_consecutive_old_packets_
           << " consecutive old packets received. Flushing the jitter buffer.";
       Flush();
       return kFlushIndicator;
     }
     return kOldPacket;
   }
 
   if (packet.codec == kVideoCodecVP9) {
-    // TODO(asapersson): Move this code to appropriate place.
-    // TODO(asapersson): Handle out of order GOF.
     if (packet.codecSpecificHeader.codecHeader.VP9.flexible_mode) {
       // TODO(asapersson): Add support for flexible mode.
       return kGeneralError;
     }
-    if (packet.codecSpecificHeader.codecHeader.VP9.ss_data_available) {
-      if (!last_gof_valid_ ||
-          IsNewerTimestamp(packet.timestamp, last_gof_timestamp_)) {
-        last_gof_.CopyGofInfoVP9(
-            packet.codecSpecificHeader.codecHeader.VP9.gof);
-        last_gof_timestamp_ = packet.timestamp;
-        last_gof_valid_ = true;
-      }
+    if (vp9_ss_map_.Insert(packet)) {
+      vp9_ss_map_.UpdateFrames(&incomplete_frames_);
     }
-    if (last_gof_valid_ &&
-        !packet.codecSpecificHeader.codecHeader.VP9.flexible_mode) {
-      uint8_t gof_idx = packet.codecSpecificHeader.codecHeader.VP9.gof_idx;
-      if (gof_idx != kNoGofIdx) {
-        if (gof_idx >= last_gof_.num_frames_in_gof) {
-          LOG(LS_WARNING) << "Incorrect gof_idx: " << gof_idx;
-          return kGeneralError;
-        }
-        RTPVideoTypeHeader* hdr = const_cast<RTPVideoTypeHeader*>(
-            &packet.codecSpecificHeader.codecHeader);
-        hdr->VP9.temporal_idx = last_gof_.temporal_idx[gof_idx];
-        hdr->VP9.temporal_up_switch = last_gof_.temporal_up_switch[gof_idx];
-      }
-    }
+    vp9_ss_map_.UpdatePacket(&packet);
+
+    if (!last_decoded_state_.in_initial_state())
+      vp9_ss_map_.RemoveOld(last_decoded_state_.time_stamp());

[stefan-webrtc, 2015/10/13 13:49:43]

Can you explain why this is needed now?

[åsapersson, 2015/10/14 15:13:50]

To not make the map possibly grow too large.

   }
 
   num_consecutive_old_packets_ = 0;
 
   VCMFrameBuffer* frame;
   FrameList* frame_list;
   const VCMFrameBufferEnum error = GetFrame(packet, &frame, &frame_list);
   if (error != kNoError)
     return error;
 
@@ skipping 618 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