New jitter buffer experiment.

webrtc/video/rtp_stream_receiver.cc

Index: webrtc/video/rtp_stream_receiver.cc
diff --git a/webrtc/video/rtp_stream_receiver.cc b/webrtc/video/rtp_stream_receiver.cc
index 576543bef682d05aa6cabef4790da3bb44fb76b7..8fa5eb45c57f629f349c751cfed77c028c2d1355 100644
--- a/webrtc/video/rtp_stream_receiver.cc
+++ b/webrtc/video/rtp_stream_receiver.cc
@@ -11,6 +11,7 @@
 #include "webrtc/video/rtp_stream_receiver.h"
 
 #include <vector>
+#include <utility>
 
 #include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
@@ -24,7 +25,11 @@
 #include "webrtc/modules/rtp_rtcp/include/rtp_receiver.h"
 #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h"
 #include "webrtc/modules/rtp_rtcp/include/ulpfec_receiver.h"
+#include "webrtc/modules/video_coding/frame_object.h"
+#include "webrtc/modules/video_coding/h264_sps_pps_tracker.h"
+#include "webrtc/modules/video_coding/packet_buffer.h"
 #include "webrtc/modules/video_coding/video_coding_impl.h"
+#include "webrtc/system_wrappers/include/field_trial.h"
 #include "webrtc/system_wrappers/include/metrics.h"
 #include "webrtc/system_wrappers/include/timestamp_extrapolator.h"
 #include "webrtc/system_wrappers/include/trace.h"
@@ -33,6 +38,11 @@
 
 namespace webrtc {
 
+namespace {
+constexpr int kPacketBufferStartSize = 32;
+constexpr int kPacketBufferMaxSixe = 2048;
+}
+
 std::unique_ptr<RtpRtcp> CreateRtpRtcpModule(
     ReceiveStatistics* receive_statistics,
     Transport* outgoing_transport,
@@ -83,7 +93,11 @@ RtpStreamReceiver::RtpStreamReceiver(
     const VideoReceiveStream::Config* config,
     ReceiveStatisticsProxy* receive_stats_proxy,
     ProcessThread* process_thread,
-    RateLimiter* retransmission_rate_limiter)
+    RateLimiter* retransmission_rate_limiter,
+    NackSender* nack_sender,
+    KeyFrameRequestSender* keyframe_request_sender,
+    video_coding::OnCompleteFrameCallback* complete_frame_callback,
+    VCMTiming* timing)
     : clock_(Clock::GetRealTimeClock()),
       config_(*config),
       video_receiver_(video_receiver),
@@ -110,7 +124,10 @@ RtpStreamReceiver::RtpStreamReceiver(
                                     remote_bitrate_estimator_,
                                     paced_sender,
                                     packet_router,
-                                    retransmission_rate_limiter)) {
+                                    retransmission_rate_limiter)),
+      complete_frame_callback_(complete_frame_callback),
+      keyframe_request_sender_(keyframe_request_sender),
+      timing_(timing) {
   packet_router_->AddRtpModule(rtp_rtcp_.get());
   rtp_receive_statistics_->RegisterRtpStatisticsCallback(receive_stats_proxy);
   rtp_receive_statistics_->RegisterRtcpStatisticsCallback(receive_stats_proxy);
@@ -192,11 +209,27 @@ RtpStreamReceiver::RtpStreamReceiver(
   rtp_rtcp_->RegisterRtcpStatisticsCallback(receive_stats_proxy);
 
   process_thread_->RegisterModule(rtp_rtcp_.get());
+
+  jitter_buffer_experiment_ =
+      field_trial::FindFullName("WebRTC-NewVideoJitterBuffer") == "Enabled";
+
+  if (jitter_buffer_experiment_) {
+    nack_module_.reset(
+        new NackModule(clock_, nack_sender, keyframe_request_sender));
+    process_thread_->RegisterModule(nack_module_.get());
+
+    packet_buffer_ = video_coding::PacketBuffer::Create(
+        clock_, kPacketBufferStartSize, kPacketBufferMaxSixe, this);
+    reference_finder_.reset(new video_coding::RtpFrameReferenceFinder(this));
+  }
 }
 
 RtpStreamReceiver::~RtpStreamReceiver() {
   process_thread_->DeRegisterModule(rtp_rtcp_.get());
 
+  if (jitter_buffer_experiment_)
+    process_thread_->DeRegisterModule(nack_module_.get());
+
   packet_router_->RemoveRtpModule(rtp_rtcp_.get());
   rtp_rtcp_->SetREMBStatus(false);
   remb_->RemoveReceiveChannel(rtp_rtcp_.get());
@@ -236,10 +269,34 @@ int32_t RtpStreamReceiver::OnReceivedPayloadData(
   WebRtcRTPHeader rtp_header_with_ntp = *rtp_header;
   rtp_header_with_ntp.ntp_time_ms =
       ntp_estimator_.Estimate(rtp_header->header.timestamp);
-  if (video_receiver_->IncomingPacket(payload_data, payload_size,
-                                      rtp_header_with_ntp) != 0) {
-    // Check this...
-    return -1;
+  if (jitter_buffer_experiment_) {
+    VCMPacket packet(payload_data, payload_size, rtp_header_with_ntp);
+    timing_->IncomingTimestamp(packet.timestamp, clock_->TimeInMilliseconds());
+    packet.timesNacked = nack_module_->OnReceivedPacket(packet);
+
+    if (packet.codec == kVideoCodecH264) {
+      switch (tracker_.CopyAndFixBitstream(&packet)) {
+        case video_coding::H264SpsPpsTracker::kRequestKeyframe:
+          keyframe_request_sender_->RequestKeyFrame();
+          FALLTHROUGH();
+        case video_coding::H264SpsPpsTracker::kDrop:
+          return 0;
+        case video_coding::H264SpsPpsTracker::kInsert:
+          break;
+      }
+    } else {
+      uint8_t* data = new uint8_t[packet.sizeBytes];
+      memcpy(data, packet.dataPtr, packet.sizeBytes);
+      packet.dataPtr = data;
+    }
+
+    packet_buffer_->InsertPacket(packet);
+  } else {
+    if (video_receiver_->IncomingPacket(payload_data, payload_size,
+                                        rtp_header_with_ntp) != 0) {
+      // Check this...
+      return -1;
+    }
   }
   return 0;
 }
@@ -357,6 +414,27 @@ int32_t RtpStreamReceiver::ResendPackets(const uint16_t* sequence_numbers,
   return rtp_rtcp_->SendNACK(sequence_numbers, length);
 }
 
+void RtpStreamReceiver::OnReceivedFrame(

[philipel, 2016/11/08 09:27:13]

When the PacketBuffer finds a complete frame this function is called.

[stefan-webrtc, 2016/11/08 10:41:34]

Can't we have RtpFrameReferenceFinder implement OnReceivedFrame instead and
bypass RtpStreamReceiver?

[nisse-webrtc, 2016/11/08 11:56:30]

I think explanations like this belong in comments in the code.

[philipel, 2016/11/08 12:28:39]

I don't think it makes sense to modify the RtpFrameReferenceFinder just to fit
this case, and also, I think this is more modular.

[philipel, 2016/11/08 12:28:39]

These comments were added to make reviewing easier, but I don't think they
should be in the code since they are very hard to maintain (documenting where
this function is called from is not good imo).

[nisse-webrtc, 2016/11/08 12:58:12]

The code comment should explain the purpose of the class. Hopefully, that can be
done without brittle references to the call sites.

I mean, the distinction between "OnReceivedFrame" and "OnCompleteFrame" is not
that obvious, in particular since "complete" is part of the explanation of the
former...

Maybe rename OnCompleteFrame to OnReadyFrame or
OnFrameWithSatisfiedDependencies?

(Note that there are two difficult unsolved computer science problems: Cache
invalidation, and how to name things). 

[philipel, 2016/11/08 13:35:51]

There are comments for OnReceivedFrameCallback (packet_buffer.h:34) and
OnCompleteFrameCallback (rtp_frame_reference_finder.h:32).

And yes, naming things are hard (FrameObject) :)

+    std::unique_ptr<video_coding::RtpFrameObject> frame) {
+  reference_finder_->ManageFrame(std::move(frame));
+}
+
+void RtpStreamReceiver::OnCompleteFrame(

[philipel, 2016/11/08 09:27:13]

When the RtpFrameReferenceFinder has found all the references for a frame this
function is called.

The |complete_frame_callback_->OnCompleteFrame| is in this case
VideoReceiveStream::OnCompleteFrame.

+    std::unique_ptr<video_coding::FrameObject> frame) {
+  {
+    rtc::CritScope lock(&last_seq_num_cs_);
+    video_coding::RtpFrameObject* rtp_frame =
+        static_cast<video_coding::RtpFrameObject*>(frame.get());
+    last_seq_num_for_pic_id_[rtp_frame->picture_id] = rtp_frame->last_seq_num();
+  }
+  complete_frame_callback_->OnCompleteFrame(std::move(frame));
+}
+
+void RtpStreamReceiver::OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) {
+  if (jitter_buffer_experiment_)
+    nack_module_->UpdateRtt(max_rtt_ms);
+}
+
 bool RtpStreamReceiver::ReceivePacket(const uint8_t* packet,
                                       size_t packet_length,
                                       const RTPHeader& header,
@@ -481,6 +559,39 @@ bool RtpStreamReceiver::DeliverRtcp(const uint8_t* rtcp_packet,
   return true;
 }
 
+void RtpStreamReceiver::FrameContinuous(uint16_t picture_id) {

[philipel, 2016/11/08 09:27:13]

This function is called from VideoReceiveStream::OnCompleteFrame after the frame
has been inserted into the FrameBuffer.

+  if (jitter_buffer_experiment_) {
+    int seq_num = -1;
+    {
+      rtc::CritScope lock(&last_seq_num_cs_);
+      auto seq_num_it = last_seq_num_for_pic_id_.find(picture_id);
+      if (seq_num_it != last_seq_num_for_pic_id_.end())
+        seq_num = seq_num_it->second;
+    }
+    if (seq_num != -1)
+      nack_module_->ClearUpTo(seq_num);
+  }
+}
+
+void RtpStreamReceiver::FrameDecoded(uint16_t picture_id) {

[philipel, 2016/11/08 09:27:13]

Called from VideoReceiveStream::Decode when a frame has been successfully
decoded.

+  if (jitter_buffer_experiment_) {
+    int seq_num = -1;
+    {
+      rtc::CritScope lock(&last_seq_num_cs_);
+      auto seq_num_it = last_seq_num_for_pic_id_.find(picture_id);
+      if (seq_num_it != last_seq_num_for_pic_id_.end()) {
+        seq_num = seq_num_it->second;
+        last_seq_num_for_pic_id_.erase(last_seq_num_for_pic_id_.begin(),
+                                       ++seq_num_it);
+      }
+    }
+    if (seq_num != -1) {
+      packet_buffer_->ClearTo(seq_num);
+      reference_finder_->ClearTo(seq_num);
+    }
+  }
+}
+
 void RtpStreamReceiver::SignalNetworkState(NetworkState state) {
   rtp_rtcp_->SetRTCPStatus(state == kNetworkUp ? config_.rtp.rtcp_mode
                                                : RtcpMode::kOff);