Fork NackModule and RtpVideoStreamReceiver

Bug: webrtc:11595
Change-Id: I4d14c0bf9c32e09d1624099a256f2778afebd4df
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175901
Commit-Queue: Tommi <tommi@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31337}

modules/video_coding/BUILD.gn

@@ -74,6 +74,8 @@ rtc_library("nack_module") {
     "histogram.h",
     "nack_module.cc",
     "nack_module.h",
+    "nack_module2.cc",
+    "nack_module2.h",
   ]
 
   deps = [
@@ -876,6 +878,7 @@ if (rtc_include_tests) {
       "jitter_buffer_unittest.cc",
       "jitter_estimator_tests.cc",
       "loss_notification_controller_unittest.cc",
+      "nack_module2_unittest.cc",
       "nack_module_unittest.cc",
       "packet_buffer_unittest.cc",
       "receiver_unittest.cc",

modules/video_coding/nack_module2.cc

@@ -0,0 +1,349 @@
+/*
+ *  Copyright (c) 2016 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 "modules/video_coding/nack_module2.h"
+
+#include <algorithm>
+#include <limits>
+
+#include "api/units/timestamp.h"
+#include "modules/utility/include/process_thread.h"
+#include "rtc_base/checks.h"
+#include "rtc_base/experiments/field_trial_parser.h"
+#include "rtc_base/logging.h"
+#include "system_wrappers/include/field_trial.h"
+
+namespace webrtc {
+
+namespace {
+const int kMaxPacketAge = 10000;
+const int kMaxNackPackets = 1000;
+const int kDefaultRttMs = 100;
+const int kMaxNackRetries = 10;
+const int kProcessFrequency = 50;
+const int kProcessIntervalMs = 1000 / kProcessFrequency;
+const int kMaxReorderedPackets = 128;
+const int kNumReorderingBuckets = 10;
+const int kDefaultSendNackDelayMs = 0;
+
+int64_t GetSendNackDelay() {
+  int64_t delay_ms = strtol(
+      webrtc::field_trial::FindFullName("WebRTC-SendNackDelayMs").c_str(),
+      nullptr, 10);
+  if (delay_ms > 0 && delay_ms <= 20) {
+    RTC_LOG(LS_INFO) << "SendNackDelay is set to " << delay_ms;
+    return delay_ms;
+  }
+  return kDefaultSendNackDelayMs;
+}
+}  // namespace
+
+NackModule2::NackInfo::NackInfo()
+    : seq_num(0), send_at_seq_num(0), sent_at_time(-1), retries(0) {}
+
+NackModule2::NackInfo::NackInfo(uint16_t seq_num,
+                                uint16_t send_at_seq_num,
+                                int64_t created_at_time)
+    : seq_num(seq_num),
+      send_at_seq_num(send_at_seq_num),
+      created_at_time(created_at_time),
+      sent_at_time(-1),
+      retries(0) {}
+
+NackModule2::BackoffSettings::BackoffSettings(TimeDelta min_retry,
+                                              TimeDelta max_rtt,
+                                              double base)
+    : min_retry_interval(min_retry), max_rtt(max_rtt), base(base) {}
+
+absl::optional<NackModule2::BackoffSettings>
+NackModule2::BackoffSettings::ParseFromFieldTrials() {
+  // Matches magic number in RTPSender::OnReceivedNack().
+  const TimeDelta kDefaultMinRetryInterval = TimeDelta::Millis(5);
+  // Upper bound on link-delay considered for exponential backoff.
+  // Selected so that cumulative delay with 1.25 base and 10 retries ends up
+  // below 3s, since above that there will be a FIR generated instead.
+  const TimeDelta kDefaultMaxRtt = TimeDelta::Millis(160);
+  // Default base for exponential backoff, adds 25% RTT delay for each retry.
+  const double kDefaultBase = 1.25;
+
+  FieldTrialParameter<bool> enabled("enabled", false);
+  FieldTrialParameter<TimeDelta> min_retry("min_retry",
+                                           kDefaultMinRetryInterval);
+  FieldTrialParameter<TimeDelta> max_rtt("max_rtt", kDefaultMaxRtt);
+  FieldTrialParameter<double> base("base", kDefaultBase);
+  ParseFieldTrial({&enabled, &min_retry, &max_rtt, &base},
+                  field_trial::FindFullName("WebRTC-ExponentialNackBackoff"));
+
+  if (enabled) {
+    return NackModule2::BackoffSettings(min_retry.Get(), max_rtt.Get(),
+                                        base.Get());
+  }
+  return absl::nullopt;
+}
+
+NackModule2::NackModule2(Clock* clock,
+                         NackSender* nack_sender,
+                         KeyFrameRequestSender* keyframe_request_sender)
+    : clock_(clock),
+      nack_sender_(nack_sender),
+      keyframe_request_sender_(keyframe_request_sender),
+      reordering_histogram_(kNumReorderingBuckets, kMaxReorderedPackets),
+      initialized_(false),
+      rtt_ms_(kDefaultRttMs),
+      newest_seq_num_(0),
+      next_process_time_ms_(-1),
+      send_nack_delay_ms_(GetSendNackDelay()),
+      backoff_settings_(BackoffSettings::ParseFromFieldTrials()) {
+  RTC_DCHECK(clock_);
+  RTC_DCHECK(nack_sender_);
+  RTC_DCHECK(keyframe_request_sender_);
+}
+
+int NackModule2::OnReceivedPacket(uint16_t seq_num, bool is_keyframe) {
+  return OnReceivedPacket(seq_num, is_keyframe, false);
+}
+
+int NackModule2::OnReceivedPacket(uint16_t seq_num,
+                                  bool is_keyframe,
+                                  bool is_recovered) {
+  rtc::CritScope lock(&crit_);
+  // TODO(philipel): When the packet includes information whether it is
+  //                 retransmitted or not, use that value instead. For
+  //                 now set it to true, which will cause the reordering
+  //                 statistics to never be updated.
+  bool is_retransmitted = true;
+
+  if (!initialized_) {
+    newest_seq_num_ = seq_num;
+    if (is_keyframe)
+      keyframe_list_.insert(seq_num);
+    initialized_ = true;
+    return 0;
+  }
+
+  // Since the |newest_seq_num_| is a packet we have actually received we know
+  // that packet has never been Nacked.
+  if (seq_num == newest_seq_num_)
+    return 0;
+
+  if (AheadOf(newest_seq_num_, seq_num)) {
+    // An out of order packet has been received.
+    auto nack_list_it = nack_list_.find(seq_num);
+    int nacks_sent_for_packet = 0;
+    if (nack_list_it != nack_list_.end()) {
+      nacks_sent_for_packet = nack_list_it->second.retries;
+      nack_list_.erase(nack_list_it);
+    }
+    if (!is_retransmitted)
+      UpdateReorderingStatistics(seq_num);
+    return nacks_sent_for_packet;
+  }
+
+  // Keep track of new keyframes.
+  if (is_keyframe)
+    keyframe_list_.insert(seq_num);
+
+  // And remove old ones so we don't accumulate keyframes.
+  auto it = keyframe_list_.lower_bound(seq_num - kMaxPacketAge);
+  if (it != keyframe_list_.begin())
+    keyframe_list_.erase(keyframe_list_.begin(), it);
+
+  if (is_recovered) {
+    recovered_list_.insert(seq_num);
+
+    // Remove old ones so we don't accumulate recovered packets.
+    auto it = recovered_list_.lower_bound(seq_num - kMaxPacketAge);
+    if (it != recovered_list_.begin())
+      recovered_list_.erase(recovered_list_.begin(), it);
+
+    // Do not send nack for packets recovered by FEC or RTX.
+    return 0;
+  }
+
+  AddPacketsToNack(newest_seq_num_ + 1, seq_num);
+  newest_seq_num_ = seq_num;
+
+  // Are there any nacks that are waiting for this seq_num.
+  std::vector<uint16_t> nack_batch = GetNackBatch(kSeqNumOnly);
+  if (!nack_batch.empty()) {
+    // This batch of NACKs is triggered externally; the initiator can
+    // batch them with other feedback messages.
+    nack_sender_->SendNack(nack_batch, /*buffering_allowed=*/true);
+  }
+
+  return 0;
+}
+
+void NackModule2::ClearUpTo(uint16_t seq_num) {
+  rtc::CritScope lock(&crit_);
+  nack_list_.erase(nack_list_.begin(), nack_list_.lower_bound(seq_num));
+  keyframe_list_.erase(keyframe_list_.begin(),
+                       keyframe_list_.lower_bound(seq_num));
+  recovered_list_.erase(recovered_list_.begin(),
+                        recovered_list_.lower_bound(seq_num));
+}
+
+void NackModule2::UpdateRtt(int64_t rtt_ms) {
+  rtc::CritScope lock(&crit_);
+  rtt_ms_ = rtt_ms;
+}
+
+void NackModule2::Clear() {
+  rtc::CritScope lock(&crit_);
+  nack_list_.clear();
+  keyframe_list_.clear();
+  recovered_list_.clear();
+}
+
+int64_t NackModule2::TimeUntilNextProcess() {
+  return std::max<int64_t>(next_process_time_ms_ - clock_->TimeInMilliseconds(),
+                           0);
+}
+
+void NackModule2::Process() {
+  if (nack_sender_) {
+    std::vector<uint16_t> nack_batch;
+    {
+      rtc::CritScope lock(&crit_);
+      nack_batch = GetNackBatch(kTimeOnly);
+    }
+
+    if (!nack_batch.empty()) {
+      // This batch of NACKs is triggered externally; there is no external
+      // initiator who can batch them with other feedback messages.
+      nack_sender_->SendNack(nack_batch, /*buffering_allowed=*/false);
+    }
+  }
+
+  // Update the next_process_time_ms_ in intervals to achieve
+  // the targeted frequency over time. Also add multiple intervals
+  // in case of a skip in time as to not make uneccessary
+  // calls to Process in order to catch up.
+  int64_t now_ms = clock_->TimeInMilliseconds();
+  if (next_process_time_ms_ == -1) {
+    next_process_time_ms_ = now_ms + kProcessIntervalMs;
+  } else {
+    next_process_time_ms_ = next_process_time_ms_ + kProcessIntervalMs +
+                            (now_ms - next_process_time_ms_) /
+                                kProcessIntervalMs * kProcessIntervalMs;
+  }
+}
+
+bool NackModule2::RemovePacketsUntilKeyFrame() {
+  while (!keyframe_list_.empty()) {
+    auto it = nack_list_.lower_bound(*keyframe_list_.begin());
+
+    if (it != nack_list_.begin()) {
+      // We have found a keyframe that actually is newer than at least one
+      // packet in the nack list.
+      nack_list_.erase(nack_list_.begin(), it);
+      return true;
+    }
+
+    // If this keyframe is so old it does not remove any packets from the list,
+    // remove it from the list of keyframes and try the next keyframe.
+    keyframe_list_.erase(keyframe_list_.begin());
+  }
+  return false;
+}
+
+void NackModule2::AddPacketsToNack(uint16_t seq_num_start,
+                                   uint16_t seq_num_end) {
+  // Remove old packets.
+  auto it = nack_list_.lower_bound(seq_num_end - kMaxPacketAge);
+  nack_list_.erase(nack_list_.begin(), it);
+
+  // If the nack list is too large, remove packets from the nack list until
+  // the latest first packet of a keyframe. If the list is still too large,
+  // clear it and request a keyframe.
+  uint16_t num_new_nacks = ForwardDiff(seq_num_start, seq_num_end);
+  if (nack_list_.size() + num_new_nacks > kMaxNackPackets) {
+    while (RemovePacketsUntilKeyFrame() &&
+           nack_list_.size() + num_new_nacks > kMaxNackPackets) {
+    }
+
+    if (nack_list_.size() + num_new_nacks > kMaxNackPackets) {
+      nack_list_.clear();
+      RTC_LOG(LS_WARNING) << "NACK list full, clearing NACK"
+                             " list and requesting keyframe.";
+      keyframe_request_sender_->RequestKeyFrame();
+      return;
+    }
+  }
+
+  for (uint16_t seq_num = seq_num_start; seq_num != seq_num_end; ++seq_num) {
+    // Do not send nack for packets that are already recovered by FEC or RTX
+    if (recovered_list_.find(seq_num) != recovered_list_.end())
+      continue;
+    NackInfo nack_info(seq_num, seq_num + WaitNumberOfPackets(0.5),
+                       clock_->TimeInMilliseconds());
+    RTC_DCHECK(nack_list_.find(seq_num) == nack_list_.end());
+    nack_list_[seq_num] = nack_info;
+  }
+}
+
+std::vector<uint16_t> NackModule2::GetNackBatch(NackFilterOptions options) {
+  bool consider_seq_num = options != kTimeOnly;
+  bool consider_timestamp = options != kSeqNumOnly;
+  Timestamp now = clock_->CurrentTime();
+  std::vector<uint16_t> nack_batch;
+  auto it = nack_list_.begin();
+  while (it != nack_list_.end()) {
+    TimeDelta resend_delay = TimeDelta::Millis(rtt_ms_);
+    if (backoff_settings_) {
+      resend_delay =
+          std::max(resend_delay, backoff_settings_->min_retry_interval);
+      if (it->second.retries > 1) {
+        TimeDelta exponential_backoff =
+            std::min(TimeDelta::Millis(rtt_ms_), backoff_settings_->max_rtt) *
+            std::pow(backoff_settings_->base, it->second.retries - 1);
+        resend_delay = std::max(resend_delay, exponential_backoff);
+      }
+    }
+
+    bool delay_timed_out =
+        now.ms() - it->second.created_at_time >= send_nack_delay_ms_;
+    bool nack_on_rtt_passed =
+        now.ms() - it->second.sent_at_time >= resend_delay.ms();
+    bool nack_on_seq_num_passed =
+        it->second.sent_at_time == -1 &&
+        AheadOrAt(newest_seq_num_, it->second.send_at_seq_num);
+    if (delay_timed_out && ((consider_seq_num && nack_on_seq_num_passed) ||
+                            (consider_timestamp && nack_on_rtt_passed))) {
+      nack_batch.emplace_back(it->second.seq_num);
+      ++it->second.retries;
+      it->second.sent_at_time = now.ms();
+      if (it->second.retries >= kMaxNackRetries) {
+        RTC_LOG(LS_WARNING) << "Sequence number " << it->second.seq_num
+                            << " removed from NACK list due to max retries.";
+        it = nack_list_.erase(it);
+      } else {
+        ++it;
+      }
+      continue;
+    }
+    ++it;
+  }
+  return nack_batch;
+}
+
+void NackModule2::UpdateReorderingStatistics(uint16_t seq_num) {
+  RTC_DCHECK(AheadOf(newest_seq_num_, seq_num));
+  uint16_t diff = ReverseDiff(newest_seq_num_, seq_num);
+  reordering_histogram_.Add(diff);
+}
+
+int NackModule2::WaitNumberOfPackets(float probability) const {
+  if (reordering_histogram_.NumValues() == 0)
+    return 0;
+  return reordering_histogram_.InverseCdf(probability);
+}
+
+}  // namespace webrtc

modules/video_coding/nack_module2.h

@@ -0,0 +1,129 @@
+/*
+ *  Copyright (c) 2016 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.
+ */
+
+#ifndef MODULES_VIDEO_CODING_NACK_MODULE2_H_
+#define MODULES_VIDEO_CODING_NACK_MODULE2_H_
+
+#include <stdint.h>
+
+#include <map>
+#include <set>
+#include <vector>
+
+#include "api/units/time_delta.h"
+#include "modules/include/module.h"
+#include "modules/include/module_common_types.h"
+#include "modules/video_coding/histogram.h"
+#include "rtc_base/critical_section.h"
+#include "rtc_base/numerics/sequence_number_util.h"
+#include "rtc_base/thread_annotations.h"
+#include "system_wrappers/include/clock.h"
+
+namespace webrtc {
+
+class NackModule2 final : public Module {
+ public:
+  NackModule2(Clock* clock,
+              NackSender* nack_sender,
+              KeyFrameRequestSender* keyframe_request_sender);
+
+  int OnReceivedPacket(uint16_t seq_num, bool is_keyframe);
+  int OnReceivedPacket(uint16_t seq_num, bool is_keyframe, bool is_recovered);
+
+  void ClearUpTo(uint16_t seq_num);
+  void UpdateRtt(int64_t rtt_ms);
+  void Clear();
+
+  // Module implementation
+  int64_t TimeUntilNextProcess() override;
+  void Process() override;
+
+ private:
+  // Which fields to consider when deciding which packet to nack in
+  // GetNackBatch.
+  enum NackFilterOptions { kSeqNumOnly, kTimeOnly, kSeqNumAndTime };
+
+  // This class holds the sequence number of the packet that is in the nack list
+  // as well as the meta data about when it should be nacked and how many times
+  // we have tried to nack this packet.
+  struct NackInfo {
+    NackInfo();
+    NackInfo(uint16_t seq_num,
+             uint16_t send_at_seq_num,
+             int64_t created_at_time);
+
+    uint16_t seq_num;
+    uint16_t send_at_seq_num;
+    int64_t created_at_time;
+    int64_t sent_at_time;
+    int retries;
+  };
+
+  struct BackoffSettings {
+    BackoffSettings(TimeDelta min_retry, TimeDelta max_rtt, double base);
+    static absl::optional<BackoffSettings> ParseFromFieldTrials();
+
+    // Min time between nacks.
+    const TimeDelta min_retry_interval;
+    // Upper bound on link-delay considered for exponential backoff.
+    const TimeDelta max_rtt;
+    // Base for the exponential backoff.
+    const double base;
+  };
+
+  void AddPacketsToNack(uint16_t seq_num_start, uint16_t seq_num_end)
+      RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_);
+
+  // Removes packets from the nack list until the next keyframe. Returns true
+  // if packets were removed.
+  bool RemovePacketsUntilKeyFrame() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_);
+  std::vector<uint16_t> GetNackBatch(NackFilterOptions options)
+      RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_);
+
+  // Update the reordering distribution.
+  void UpdateReorderingStatistics(uint16_t seq_num)
+      RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_);
+
+  // Returns how many packets we have to wait in order to receive the packet
+  // with probability |probabilty| or higher.
+  int WaitNumberOfPackets(float probability) const
+      RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_);
+
+  rtc::CriticalSection crit_;
+  Clock* const clock_;
+  NackSender* const nack_sender_;
+  KeyFrameRequestSender* const keyframe_request_sender_;
+
+  // TODO(philipel): Some of the variables below are consistently used on a
+  // known thread (e.g. see |initialized_|). Those probably do not need
+  // synchronized access.
+  std::map<uint16_t, NackInfo, DescendingSeqNumComp<uint16_t>> nack_list_
+      RTC_GUARDED_BY(crit_);
+  std::set<uint16_t, DescendingSeqNumComp<uint16_t>> keyframe_list_
+      RTC_GUARDED_BY(crit_);
+  std::set<uint16_t, DescendingSeqNumComp<uint16_t>> recovered_list_
+      RTC_GUARDED_BY(crit_);
+  video_coding::Histogram reordering_histogram_ RTC_GUARDED_BY(crit_);
+  bool initialized_ RTC_GUARDED_BY(crit_);
+  int64_t rtt_ms_ RTC_GUARDED_BY(crit_);
+  uint16_t newest_seq_num_ RTC_GUARDED_BY(crit_);
+
+  // Only touched on the process thread.
+  int64_t next_process_time_ms_;
+
+  // Adds a delay before send nack on packet received.
+  const int64_t send_nack_delay_ms_;
+
+  const absl::optional<BackoffSettings> backoff_settings_;
+};
+
+}  // namespace webrtc
+
+#endif  // MODULES_VIDEO_CODING_NACK_MODULE2_H_

modules/video_coding/nack_module2_unittest.cc

@@ -0,0 +1,371 @@
+/*
+ *  Copyright (c) 2016 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 "modules/video_coding/nack_module2.h"
+
+#include <algorithm>
+#include <cstdint>
+#include <cstring>
+#include <memory>
+
+#include "system_wrappers/include/clock.h"
+#include "test/field_trial.h"
+#include "test/gtest.h"
+
+namespace webrtc {
+class TestNackModule2 : public ::testing::TestWithParam<bool>,
+                        public NackSender,
+                        public KeyFrameRequestSender {
+ protected:
+  TestNackModule2()
+      : clock_(new SimulatedClock(0)),
+        field_trial_(GetParam()
+                         ? "WebRTC-ExponentialNackBackoff/enabled:true/"
+                         : "WebRTC-ExponentialNackBackoff/enabled:false/"),
+        nack_module_(clock_.get(), this, this),
+        keyframes_requested_(0) {}
+
+  void SetUp() override { nack_module_.UpdateRtt(kDefaultRttMs); }
+
+  void SendNack(const std::vector<uint16_t>& sequence_numbers,
+                bool buffering_allowed) override {
+    sent_nacks_.insert(sent_nacks_.end(), sequence_numbers.begin(),
+                       sequence_numbers.end());
+  }
+
+  void RequestKeyFrame() override { ++keyframes_requested_; }
+
+  static constexpr int64_t kDefaultRttMs = 20;
+  std::unique_ptr<SimulatedClock> clock_;
+  test::ScopedFieldTrials field_trial_;
+  NackModule2 nack_module_;
+  std::vector<uint16_t> sent_nacks_;
+  int keyframes_requested_;
+};
+
+TEST_P(TestNackModule2, NackOnePacket) {
+  nack_module_.OnReceivedPacket(1, false, false);
+  nack_module_.OnReceivedPacket(3, false, false);
+  EXPECT_EQ(1u, sent_nacks_.size());
+  EXPECT_EQ(2, sent_nacks_[0]);
+}
+
+TEST_P(TestNackModule2, WrappingSeqNum) {
+  nack_module_.OnReceivedPacket(0xfffe, false, false);
+  nack_module_.OnReceivedPacket(1, false, false);
+  EXPECT_EQ(2u, sent_nacks_.size());
+  EXPECT_EQ(0xffff, sent_nacks_[0]);
+  EXPECT_EQ(0, sent_nacks_[1]);
+}
+
+TEST_P(TestNackModule2, WrappingSeqNumClearToKeyframe) {
+  nack_module_.OnReceivedPacket(0xfffe, false, false);
+  nack_module_.OnReceivedPacket(1, false, false);
+  EXPECT_EQ(2u, sent_nacks_.size());
+  EXPECT_EQ(0xffff, sent_nacks_[0]);
+  EXPECT_EQ(0, sent_nacks_[1]);
+
+  sent_nacks_.clear();
+  nack_module_.OnReceivedPacket(2, true, false);
+  EXPECT_EQ(0u, sent_nacks_.size());
+
+  nack_module_.OnReceivedPacket(501, true, false);
+  EXPECT_EQ(498u, sent_nacks_.size());
+  for (int seq_num = 3; seq_num < 501; ++seq_num)
+    EXPECT_EQ(seq_num, sent_nacks_[seq_num - 3]);
+
+  sent_nacks_.clear();
+  nack_module_.OnReceivedPacket(1001, false, false);
+  EXPECT_EQ(499u, sent_nacks_.size());
+  for (int seq_num = 502; seq_num < 1001; ++seq_num)
+    EXPECT_EQ(seq_num, sent_nacks_[seq_num - 502]);
+
+  sent_nacks_.clear();
+  clock_->AdvanceTimeMilliseconds(100);
+  nack_module_.Process();
+  EXPECT_EQ(999u, sent_nacks_.size());
+  EXPECT_EQ(0xffff, sent_nacks_[0]);
+  EXPECT_EQ(0, sent_nacks_[1]);
+  for (int seq_num = 3; seq_num < 501; ++seq_num)
+    EXPECT_EQ(seq_num, sent_nacks_[seq_num - 1]);
+  for (int seq_num = 502; seq_num < 1001; ++seq_num)
+    EXPECT_EQ(seq_num, sent_nacks_[seq_num - 2]);
+
+  // Adding packet 1004 will cause the nack list to reach it's max limit.
+  // It will then clear all nacks up to the next keyframe (seq num 2),
+  // thus removing 0xffff and 0 from the nack list.
+  sent_nacks_.clear();
+  nack_module_.OnReceivedPacket(1004, false, false);
+  EXPECT_EQ(2u, sent_nacks_.size());
+  EXPECT_EQ(1002, sent_nacks_[0]);
+  EXPECT_EQ(1003, sent_nacks_[1]);
+
+  sent_nacks_.clear();
+  clock_->AdvanceTimeMilliseconds(100);
+  nack_module_.Process();
+  EXPECT_EQ(999u, sent_nacks_.size());
+  for (int seq_num = 3; seq_num < 501; ++seq_num)
+    EXPECT_EQ(seq_num, sent_nacks_[seq_num - 3]);
+  for (int seq_num = 502; seq_num < 1001; ++seq_num)
+    EXPECT_EQ(seq_num, sent_nacks_[seq_num - 4]);
+
+  // Adding packet 1007 will cause the nack module to overflow again, thus
+  // clearing everything up to 501 which is the next keyframe.
+  nack_module_.OnReceivedPacket(1007, false, false);
+  sent_nacks_.clear();
+  clock_->AdvanceTimeMilliseconds(100);
+  nack_module_.Process();
+  EXPECT_EQ(503u, sent_nacks_.size());
+  for (int seq_num = 502; seq_num < 1001; ++seq_num)
+    EXPECT_EQ(seq_num, sent_nacks_[seq_num - 502]);
+  EXPECT_EQ(1005, sent_nacks_[501]);
+  EXPECT_EQ(1006, sent_nacks_[502]);
+}
+
+TEST_P(TestNackModule2, DontBurstOnTimeSkip) {
+  nack_module_.Process();
+  clock_->AdvanceTimeMilliseconds(20);
+  EXPECT_EQ(0, nack_module_.TimeUntilNextProcess());
+  nack_module_.Process();
+
+  clock_->AdvanceTimeMilliseconds(100);
+  EXPECT_EQ(0, nack_module_.TimeUntilNextProcess());
+  nack_module_.Process();
+  EXPECT_EQ(20, nack_module_.TimeUntilNextProcess());
+
+  clock_->AdvanceTimeMilliseconds(19);
+  EXPECT_EQ(1, nack_module_.TimeUntilNextProcess());
+  clock_->AdvanceTimeMilliseconds(2);
+  nack_module_.Process();
+  EXPECT_EQ(19, nack_module_.TimeUntilNextProcess());
+
+  clock_->AdvanceTimeMilliseconds(19);
+  EXPECT_EQ(0, nack_module_.TimeUntilNextProcess());
+  nack_module_.Process();
+
+  clock_->AdvanceTimeMilliseconds(21);
+  EXPECT_EQ(0, nack_module_.TimeUntilNextProcess());
+  nack_module_.Process();
+  EXPECT_EQ(19, nack_module_.TimeUntilNextProcess());
+}
+
+TEST_P(TestNackModule2, ResendNack) {
+  nack_module_.OnReceivedPacket(1, false, false);
+  nack_module_.OnReceivedPacket(3, false, false);
+  size_t expected_nacks_sent = 1;
+  EXPECT_EQ(expected_nacks_sent, sent_nacks_.size());
+  EXPECT_EQ(2, sent_nacks_[0]);
+
+  if (GetParam()) {
+    // Retry has to wait at least 5ms by default.
+    nack_module_.UpdateRtt(1);
+    clock_->AdvanceTimeMilliseconds(4);
+    nack_module_.Process();  // Too early.
+    EXPECT_EQ(expected_nacks_sent, sent_nacks_.size());
+
+    clock_->AdvanceTimeMilliseconds(1);
+    nack_module_.Process();  // Now allowed.
+    EXPECT_EQ(++expected_nacks_sent, sent_nacks_.size());
+  } else {
+    nack_module_.UpdateRtt(1);
+    clock_->AdvanceTimeMilliseconds(1);
+    nack_module_.Process();  // Fast retransmit allowed.
+    EXPECT_EQ(++expected_nacks_sent, sent_nacks_.size());
+  }
+
+  // N:th try has to wait b^(N-1) * rtt by default.
+  const double b = GetParam() ? 1.25 : 1.0;
+  for (int i = 2; i < 10; ++i) {
+    // Change RTT, above the 40ms max for exponential backoff.
+    TimeDelta rtt = TimeDelta::Millis(160);  // + (i * 10 - 40)
+    nack_module_.UpdateRtt(rtt.ms());
+
+    // RTT gets capped at 160ms in backoff calculations.
+    TimeDelta expected_backoff_delay =
+        std::pow(b, i - 1) * std::min(rtt, TimeDelta::Millis(160));
+
+    // Move to one millisecond before next allowed NACK.
+    clock_->AdvanceTimeMilliseconds(expected_backoff_delay.ms() - 1);
+    nack_module_.Process();
+    EXPECT_EQ(expected_nacks_sent, sent_nacks_.size());
+
+    // Move to one millisecond after next allowed NACK.
+    // After rather than on to avoid rounding errors.
+    clock_->AdvanceTimeMilliseconds(2);
+    nack_module_.Process();  // Now allowed.
+    EXPECT_EQ(++expected_nacks_sent, sent_nacks_.size());
+  }
+
+  // Giving up after 10 tries.
+  clock_->AdvanceTimeMilliseconds(3000);
+  nack_module_.Process();
+  EXPECT_EQ(expected_nacks_sent, sent_nacks_.size());
+}
+
+TEST_P(TestNackModule2, ResendPacketMaxRetries) {
+  nack_module_.OnReceivedPacket(1, false, false);
+  nack_module_.OnReceivedPacket(3, false, false);
+  EXPECT_EQ(1u, sent_nacks_.size());
+  EXPECT_EQ(2, sent_nacks_[0]);
+
+  int backoff_factor = 1;
+  for (size_t retries = 1; retries < 10; ++retries) {
+    // Exponential backoff, so that we don't reject NACK because of time.
+    clock_->AdvanceTimeMilliseconds(backoff_factor * kDefaultRttMs);
+    backoff_factor *= 2;
+    nack_module_.Process();
+    EXPECT_EQ(retries + 1, sent_nacks_.size());
+  }
+
+  clock_->AdvanceTimeMilliseconds(backoff_factor * kDefaultRttMs);
+  nack_module_.Process();
+  EXPECT_EQ(10u, sent_nacks_.size());
+}
+
+TEST_P(TestNackModule2, TooLargeNackList) {
+  nack_module_.OnReceivedPacket(0, false, false);
+  nack_module_.OnReceivedPacket(1001, false, false);
+  EXPECT_EQ(1000u, sent_nacks_.size());
+  EXPECT_EQ(0, keyframes_requested_);
+  nack_module_.OnReceivedPacket(1003, false, false);
+  EXPECT_EQ(1000u, sent_nacks_.size());
+  EXPECT_EQ(1, keyframes_requested_);
+  nack_module_.OnReceivedPacket(1004, false, false);
+  EXPECT_EQ(1000u, sent_nacks_.size());
+  EXPECT_EQ(1, keyframes_requested_);
+}
+
+TEST_P(TestNackModule2, TooLargeNackListWithKeyFrame) {
+  nack_module_.OnReceivedPacket(0, false, false);
+  nack_module_.OnReceivedPacket(1, true, false);
+  nack_module_.OnReceivedPacket(1001, false, false);
+  EXPECT_EQ(999u, sent_nacks_.size());
+  EXPECT_EQ(0, keyframes_requested_);
+  nack_module_.OnReceivedPacket(1003, false, false);
+  EXPECT_EQ(1000u, sent_nacks_.size());
+  EXPECT_EQ(0, keyframes_requested_);
+  nack_module_.OnReceivedPacket(1005, false, false);
+  EXPECT_EQ(1000u, sent_nacks_.size());
+  EXPECT_EQ(1, keyframes_requested_);
+}
+
+TEST_P(TestNackModule2, ClearUpTo) {
+  nack_module_.OnReceivedPacket(0, false, false);
+  nack_module_.OnReceivedPacket(100, false, false);
+  EXPECT_EQ(99u, sent_nacks_.size());
+
+  sent_nacks_.clear();
+  clock_->AdvanceTimeMilliseconds(100);
+  nack_module_.ClearUpTo(50);
+  nack_module_.Process();
+  EXPECT_EQ(50u, sent_nacks_.size());
+  EXPECT_EQ(50, sent_nacks_[0]);
+}
+
+TEST_P(TestNackModule2, ClearUpToWrap) {
+  nack_module_.OnReceivedPacket(0xfff0, false, false);
+  nack_module_.OnReceivedPacket(0xf, false, false);
+  EXPECT_EQ(30u, sent_nacks_.size());
+
+  sent_nacks_.clear();
+  clock_->AdvanceTimeMilliseconds(100);
+  nack_module_.ClearUpTo(0);
+  nack_module_.Process();
+  EXPECT_EQ(15u, sent_nacks_.size());
+  EXPECT_EQ(0, sent_nacks_[0]);
+}
+
+TEST_P(TestNackModule2, PacketNackCount) {
+  EXPECT_EQ(0, nack_module_.OnReceivedPacket(0, false, false));
+  EXPECT_EQ(0, nack_module_.OnReceivedPacket(2, false, false));
+  EXPECT_EQ(1, nack_module_.OnReceivedPacket(1, false, false));
+
+  sent_nacks_.clear();
+  nack_module_.UpdateRtt(100);
+  EXPECT_EQ(0, nack_module_.OnReceivedPacket(5, false, false));
+  clock_->AdvanceTimeMilliseconds(100);
+  nack_module_.Process();
+  clock_->AdvanceTimeMilliseconds(125);
+  nack_module_.Process();
+  EXPECT_EQ(3, nack_module_.OnReceivedPacket(3, false, false));
+  EXPECT_EQ(3, nack_module_.OnReceivedPacket(4, false, false));
+  EXPECT_EQ(0, nack_module_.OnReceivedPacket(4, false, false));
+}
+
+TEST_P(TestNackModule2, NackListFullAndNoOverlapWithKeyframes) {
+  const int kMaxNackPackets = 1000;
+  const unsigned int kFirstGap = kMaxNackPackets - 20;
+  const unsigned int kSecondGap = 200;
+  uint16_t seq_num = 0;
+  nack_module_.OnReceivedPacket(seq_num++, true, false);
+  seq_num += kFirstGap;
+  nack_module_.OnReceivedPacket(seq_num++, true, false);
+  EXPECT_EQ(kFirstGap, sent_nacks_.size());
+  sent_nacks_.clear();
+  seq_num += kSecondGap;
+  nack_module_.OnReceivedPacket(seq_num, true, false);
+  EXPECT_EQ(kSecondGap, sent_nacks_.size());
+}
+
+TEST_P(TestNackModule2, HandleFecRecoveredPacket) {
+  nack_module_.OnReceivedPacket(1, false, false);
+  nack_module_.OnReceivedPacket(4, false, true);
+  EXPECT_EQ(0u, sent_nacks_.size());
+  nack_module_.OnReceivedPacket(5, false, false);
+  EXPECT_EQ(2u, sent_nacks_.size());
+}
+
+TEST_P(TestNackModule2, SendNackWithoutDelay) {
+  nack_module_.OnReceivedPacket(0, false, false);
+  nack_module_.OnReceivedPacket(100, false, false);
+  EXPECT_EQ(99u, sent_nacks_.size());
+}
+
+INSTANTIATE_TEST_SUITE_P(WithAndWithoutBackoff,
+                         TestNackModule2,
+                         ::testing::Values(true, false));
+
+class TestNackModule2WithFieldTrial : public ::testing::Test,
+                                      public NackSender,
+                                      public KeyFrameRequestSender {
+ protected:
+  TestNackModule2WithFieldTrial()
+      : nack_delay_field_trial_("WebRTC-SendNackDelayMs/10/"),
+        clock_(new SimulatedClock(0)),
+        nack_module_(clock_.get(), this, this),
+        keyframes_requested_(0) {}
+
+  void SendNack(const std::vector<uint16_t>& sequence_numbers,
+                bool buffering_allowed) override {
+    sent_nacks_.insert(sent_nacks_.end(), sequence_numbers.begin(),
+                       sequence_numbers.end());
+  }
+
+  void RequestKeyFrame() override { ++keyframes_requested_; }
+
+  test::ScopedFieldTrials nack_delay_field_trial_;
+  std::unique_ptr<SimulatedClock> clock_;
+  NackModule2 nack_module_;
+  std::vector<uint16_t> sent_nacks_;
+  int keyframes_requested_;
+};
+
+TEST_F(TestNackModule2WithFieldTrial, SendNackWithDelay) {
+  nack_module_.OnReceivedPacket(0, false, false);
+  nack_module_.OnReceivedPacket(100, false, false);
+  EXPECT_EQ(0u, sent_nacks_.size());
+  clock_->AdvanceTimeMilliseconds(10);
+  nack_module_.OnReceivedPacket(106, false, false);
+  EXPECT_EQ(99u, sent_nacks_.size());
+  clock_->AdvanceTimeMilliseconds(10);
+  nack_module_.OnReceivedPacket(109, false, false);
+  EXPECT_EQ(104u, sent_nacks_.size());
+}
+}  // namespace webrtc

video/BUILD.gn

@@ -34,6 +34,8 @@ rtc_library("video") {
     "rtp_streams_synchronizer2.h",
     "rtp_video_stream_receiver.cc",
     "rtp_video_stream_receiver.h",
+    "rtp_video_stream_receiver2.cc",
+    "rtp_video_stream_receiver2.h",
     "rtp_video_stream_receiver_frame_transformer_delegate.cc",
     "rtp_video_stream_receiver_frame_transformer_delegate.h",
     "send_delay_stats.cc",
@@ -528,6 +530,7 @@ if (rtc_include_tests) {
       "receive_statistics_proxy2_unittest.cc",
       "receive_statistics_proxy_unittest.cc",
       "report_block_stats_unittest.cc",
+      "rtp_video_stream_receiver2_unittest.cc",
       "rtp_video_stream_receiver_frame_transformer_delegate_unittest.cc",
       "rtp_video_stream_receiver_unittest.cc",
       "send_delay_stats_unittest.cc",

video/rtp_video_stream_receiver.cc

@@ -77,8 +77,6 @@ int PacketBufferMaxSize() {
   return packet_buffer_max_size;
 }
 
-}  // namespace
-
 std::unique_ptr<RtpRtcp> CreateRtpRtcpModule(
     Clock* clock,
     ReceiveStatistics* receive_statistics,
@@ -107,6 +105,8 @@ std::unique_ptr<RtpRtcp> CreateRtpRtcpModule(
 
 static const int kPacketLogIntervalMs = 10000;
 
+}  // namespace
+
 RtpVideoStreamReceiver::RtcpFeedbackBuffer::RtcpFeedbackBuffer(
     KeyFrameRequestSender* key_frame_request_sender,
     NackSender* nack_sender,

video/rtp_video_stream_receiver.h

@@ -69,7 +69,8 @@ class RtpVideoStreamReceiver : public LossNotificationSender,
                                public KeyFrameRequestSender,
                                public video_coding::OnCompleteFrameCallback,
                                public OnDecryptedFrameCallback,
-                               public OnDecryptionStatusChangeCallback {
+                               public OnDecryptionStatusChangeCallback,
+                               public RtpVideoFrameReceiver {
  public:
   // DEPRECATED due to dependency on ReceiveStatisticsProxy.
   RtpVideoStreamReceiver(
@@ -205,9 +206,11 @@ class RtpVideoStreamReceiver : public LossNotificationSender,
   void AddSecondarySink(RtpPacketSinkInterface* sink);
   void RemoveSecondarySink(const RtpPacketSinkInterface* sink);
 
-  virtual void ManageFrame(std::unique_ptr<video_coding::RtpFrameObject> frame);
-
  private:
+  // Implements RtpVideoFrameReceiver.
+  void ManageFrame(
+      std::unique_ptr<video_coding::RtpFrameObject> frame) override;
+
   // Used for buffering RTCP feedback messages and sending them all together.
   // Note:
   // 1. Key frame requests and NACKs are mutually exclusive, with the

video/rtp_video_stream_receiver2.h

@@ -0,0 +1,386 @@
+/*
+ *  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.
+ */
+
+#ifndef VIDEO_RTP_VIDEO_STREAM_RECEIVER2_H_
+#define VIDEO_RTP_VIDEO_STREAM_RECEIVER2_H_
+
+#include <atomic>
+#include <list>
+#include <map>
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "absl/types/optional.h"
+#include "api/array_view.h"
+#include "api/crypto/frame_decryptor_interface.h"
+#include "api/video/color_space.h"
+#include "api/video_codecs/video_codec.h"
+#include "call/rtp_packet_sink_interface.h"
+#include "call/syncable.h"
+#include "call/video_receive_stream.h"
+#include "modules/rtp_rtcp/include/receive_statistics.h"
+#include "modules/rtp_rtcp/include/remote_ntp_time_estimator.h"
+#include "modules/rtp_rtcp/include/rtp_header_extension_map.h"
+#include "modules/rtp_rtcp/include/rtp_rtcp.h"
+#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
+#include "modules/rtp_rtcp/source/absolute_capture_time_receiver.h"
+#include "modules/rtp_rtcp/source/rtp_dependency_descriptor_extension.h"
+#include "modules/rtp_rtcp/source/rtp_packet_received.h"
+#include "modules/rtp_rtcp/source/rtp_video_header.h"
+#include "modules/rtp_rtcp/source/video_rtp_depacketizer.h"
+#include "modules/video_coding/h264_sps_pps_tracker.h"
+#include "modules/video_coding/loss_notification_controller.h"
+#include "modules/video_coding/packet_buffer.h"
+#include "modules/video_coding/rtp_frame_reference_finder.h"
+#include "modules/video_coding/unique_timestamp_counter.h"
+#include "rtc_base/constructor_magic.h"
+#include "rtc_base/critical_section.h"
+#include "rtc_base/experiments/field_trial_parser.h"
+#include "rtc_base/numerics/sequence_number_util.h"
+#include "rtc_base/synchronization/sequence_checker.h"
+#include "rtc_base/thread_annotations.h"
+#include "rtc_base/thread_checker.h"
+#include "video/buffered_frame_decryptor.h"
+#include "video/rtp_video_stream_receiver_frame_transformer_delegate.h"
+
+namespace webrtc {
+
+class NackModule2;
+class PacketRouter;
+class ProcessThread;
+class ReceiveStatistics;
+class RtcpRttStats;
+class RtpPacketReceived;
+class Transport;
+class UlpfecReceiver;
+
+class RtpVideoStreamReceiver2 : public LossNotificationSender,
+                                public RecoveredPacketReceiver,
+                                public RtpPacketSinkInterface,
+                                public KeyFrameRequestSender,
+                                public video_coding::OnCompleteFrameCallback,
+                                public OnDecryptedFrameCallback,
+                                public OnDecryptionStatusChangeCallback,
+                                public RtpVideoFrameReceiver {
+ public:
+  RtpVideoStreamReceiver2(
+      Clock* clock,
+      Transport* transport,
+      RtcpRttStats* rtt_stats,
+      // The packet router is optional; if provided, the RtpRtcp module for this
+      // stream is registered as a candidate for sending REMB and transport
+      // feedback.
+      PacketRouter* packet_router,
+      const VideoReceiveStream::Config* config,
+      ReceiveStatistics* rtp_receive_statistics,
+      RtcpPacketTypeCounterObserver* rtcp_packet_type_counter_observer,
+      RtcpCnameCallback* rtcp_cname_callback,
+      ProcessThread* process_thread,
+      NackSender* nack_sender,
+      // The KeyFrameRequestSender is optional; if not provided, key frame
+      // requests are sent via the internal RtpRtcp module.
+      KeyFrameRequestSender* keyframe_request_sender,
+      video_coding::OnCompleteFrameCallback* complete_frame_callback,
+      rtc::scoped_refptr<FrameDecryptorInterface> frame_decryptor,
+      rtc::scoped_refptr<FrameTransformerInterface> frame_transformer);
+  ~RtpVideoStreamReceiver2() override;
+
+  void AddReceiveCodec(const VideoCodec& video_codec,
+                       const std::map<std::string, std::string>& codec_params,
+                       bool raw_payload);
+
+  void StartReceive();
+  void StopReceive();
+
+  // Produces the transport-related timestamps; current_delay_ms is left unset.
+  absl::optional<Syncable::Info> GetSyncInfo() const;
+
+  bool DeliverRtcp(const uint8_t* rtcp_packet, size_t rtcp_packet_length);
+
+  void FrameContinuous(int64_t seq_num);
+
+  void FrameDecoded(int64_t seq_num);
+
+  void SignalNetworkState(NetworkState state);
+
+  // Returns number of different frames seen.
+  int GetUniqueFramesSeen() const {
+    RTC_DCHECK_RUN_ON(&worker_task_checker_);
+    return frame_counter_.GetUniqueSeen();
+  }
+
+  // Implements RtpPacketSinkInterface.
+  void OnRtpPacket(const RtpPacketReceived& packet) override;
+
+  // TODO(philipel): Stop using VCMPacket in the new jitter buffer and then
+  //                 remove this function. Public only for tests.
+  void OnReceivedPayloadData(rtc::CopyOnWriteBuffer codec_payload,
+                             const RtpPacketReceived& rtp_packet,
+                             const RTPVideoHeader& video);
+
+  // Implements RecoveredPacketReceiver.
+  void OnRecoveredPacket(const uint8_t* packet, size_t packet_length) override;
+
+  // Send an RTCP keyframe request.
+  void RequestKeyFrame() override;
+
+  // Implements LossNotificationSender.
+  void SendLossNotification(uint16_t last_decoded_seq_num,
+                            uint16_t last_received_seq_num,
+                            bool decodability_flag,
+                            bool buffering_allowed) override;
+
+  bool IsUlpfecEnabled() const;
+  bool IsRetransmissionsEnabled() const;
+
+  // Returns true if a decryptor is attached and frames can be decrypted.
+  // Updated by OnDecryptionStatusChangeCallback. Note this refers to Frame
+  // Decryption not SRTP.
+  bool IsDecryptable() const;
+
+  // Don't use, still experimental.
+  void RequestPacketRetransmit(const std::vector<uint16_t>& sequence_numbers);
+
+  // Implements OnCompleteFrameCallback.
+  void OnCompleteFrame(
+      std::unique_ptr<video_coding::EncodedFrame> frame) override;
+
+  // Implements OnDecryptedFrameCallback.
+  void OnDecryptedFrame(
+      std::unique_ptr<video_coding::RtpFrameObject> frame) override;
+
+  // Implements OnDecryptionStatusChangeCallback.
+  void OnDecryptionStatusChange(
+      FrameDecryptorInterface::Status status) override;
+
+  // Optionally set a frame decryptor after a stream has started. This will not
+  // reset the decoder state.
+  void SetFrameDecryptor(
+      rtc::scoped_refptr<FrameDecryptorInterface> frame_decryptor);
+
+  // Sets a frame transformer after a stream has started, if no transformer
+  // has previously been set. Does not reset the decoder state.
+  void SetDepacketizerToDecoderFrameTransformer(
+      rtc::scoped_refptr<FrameTransformerInterface> frame_transformer);
+
+  // Called by VideoReceiveStream when stats are updated.
+  void UpdateRtt(int64_t max_rtt_ms);
+
+  absl::optional<int64_t> LastReceivedPacketMs() const;
+  absl::optional<int64_t> LastReceivedKeyframePacketMs() const;
+
+  // RtpDemuxer only forwards a given RTP packet to one sink. However, some
+  // sinks, such as FlexFEC, might wish to be informed of all of the packets
+  // a given sink receives (or any set of sinks). They may do so by registering
+  // themselves as secondary sinks.
+  void AddSecondarySink(RtpPacketSinkInterface* sink);
+  void RemoveSecondarySink(const RtpPacketSinkInterface* sink);
+
+ private:
+  // Implements RtpVideoFrameReceiver.
+  void ManageFrame(
+      std::unique_ptr<video_coding::RtpFrameObject> frame) override;
+
+  // Used for buffering RTCP feedback messages and sending them all together.
+  // Note:
+  // 1. Key frame requests and NACKs are mutually exclusive, with the
+  //    former taking precedence over the latter.
+  // 2. Loss notifications are orthogonal to either. (That is, may be sent
+  //    alongside either.)
+  class RtcpFeedbackBuffer : public KeyFrameRequestSender,
+                             public NackSender,
+                             public LossNotificationSender {
+   public:
+    RtcpFeedbackBuffer(KeyFrameRequestSender* key_frame_request_sender,
+                       NackSender* nack_sender,
+                       LossNotificationSender* loss_notification_sender);
+
+    ~RtcpFeedbackBuffer() override = default;
+
+    // KeyFrameRequestSender implementation.
+    void RequestKeyFrame() RTC_LOCKS_EXCLUDED(cs_) override;
+
+    // NackSender implementation.
+    void SendNack(const std::vector<uint16_t>& sequence_numbers,
+                  bool buffering_allowed) RTC_LOCKS_EXCLUDED(cs_) override;
+
+    // LossNotificationSender implementation.
+    void SendLossNotification(uint16_t last_decoded_seq_num,
+                              uint16_t last_received_seq_num,
+                              bool decodability_flag,
+                              bool buffering_allowed)
+        RTC_LOCKS_EXCLUDED(cs_) override;
+
+    // Send all RTCP feedback messages buffered thus far.
+    void SendBufferedRtcpFeedback() RTC_LOCKS_EXCLUDED(cs_);
+
+   private:
+    // LNTF-related state.
+    struct LossNotificationState {
+      LossNotificationState(uint16_t last_decoded_seq_num,
+                            uint16_t last_received_seq_num,
+                            bool decodability_flag)
+          : last_decoded_seq_num(last_decoded_seq_num),
+            last_received_seq_num(last_received_seq_num),
+            decodability_flag(decodability_flag) {}
+
+      uint16_t last_decoded_seq_num;
+      uint16_t last_received_seq_num;
+      bool decodability_flag;
+    };
+    struct ConsumedRtcpFeedback {
+      bool request_key_frame = false;
+      std::vector<uint16_t> nack_sequence_numbers;
+      absl::optional<LossNotificationState> lntf_state;
+    };
+
+    ConsumedRtcpFeedback ConsumeRtcpFeedback() RTC_LOCKS_EXCLUDED(cs_);
+    ConsumedRtcpFeedback ConsumeRtcpFeedbackLocked()
+        RTC_EXCLUSIVE_LOCKS_REQUIRED(cs_);
+    // This method is called both with and without cs_ held.
+    void SendRtcpFeedback(ConsumedRtcpFeedback feedback);
+
+    KeyFrameRequestSender* const key_frame_request_sender_;
+    NackSender* const nack_sender_;
+    LossNotificationSender* const loss_notification_sender_;
+
+    // NACKs are accessible from two threads due to nack_module_ being a module.
+    rtc::CriticalSection cs_;
+
+    // Key-frame-request-related state.
+    bool request_key_frame_ RTC_GUARDED_BY(cs_);
+
+    // NACK-related state.
+    std::vector<uint16_t> nack_sequence_numbers_ RTC_GUARDED_BY(cs_);
+
+    absl::optional<LossNotificationState> lntf_state_ RTC_GUARDED_BY(cs_);
+  };
+  enum ParseGenericDependenciesResult {
+    kDropPacket,
+    kHasGenericDescriptor,
+    kNoGenericDescriptor
+  };
+
+  // Entry point doing non-stats work for a received packet. Called
+  // for the same packet both before and after RED decapsulation.
+  void ReceivePacket(const RtpPacketReceived& packet);
+  // Parses and handles RED headers.
+  // This function assumes that it's being called from only one thread.
+  void ParseAndHandleEncapsulatingHeader(const RtpPacketReceived& packet);
+  void NotifyReceiverOfEmptyPacket(uint16_t seq_num);
+  void UpdateHistograms();
+  bool IsRedEnabled() const;
+  void InsertSpsPpsIntoTracker(uint8_t payload_type);
+  void OnInsertedPacket(video_coding::PacketBuffer::InsertResult result);
+  ParseGenericDependenciesResult ParseGenericDependenciesExtension(
+      const RtpPacketReceived& rtp_packet,
+      RTPVideoHeader* video_header) RTC_RUN_ON(worker_task_checker_);
+  void OnAssembledFrame(std::unique_ptr<video_coding::RtpFrameObject> frame);
+
+  Clock* const clock_;
+  // Ownership of this object lies with VideoReceiveStream, which owns |this|.
+  const VideoReceiveStream::Config& config_;
+  PacketRouter* const packet_router_;
+  ProcessThread* const process_thread_;
+
+  RemoteNtpTimeEstimator ntp_estimator_;
+
+  RtpHeaderExtensionMap rtp_header_extensions_;
+  // Set by the field trial WebRTC-ForcePlayoutDelay to override any playout
+  // delay that is specified in the received packets.
+  FieldTrialOptional<int> forced_playout_delay_max_ms_;
+  FieldTrialOptional<int> forced_playout_delay_min_ms_;
+  ReceiveStatistics* const rtp_receive_statistics_;
+  std::unique_ptr<UlpfecReceiver> ulpfec_receiver_;
+
+  SequenceChecker worker_task_checker_;
+  bool receiving_ RTC_GUARDED_BY(worker_task_checker_);
+  int64_t last_packet_log_ms_ RTC_GUARDED_BY(worker_task_checker_);
+
+  const std::unique_ptr<RtpRtcp> rtp_rtcp_;
+
+  video_coding::OnCompleteFrameCallback* complete_frame_callback_;
+  KeyFrameRequestSender* const keyframe_request_sender_;
+
+  RtcpFeedbackBuffer rtcp_feedback_buffer_;
+  std::unique_ptr<NackModule2> nack_module_;
+  std::unique_ptr<LossNotificationController> loss_notification_controller_;
+
+  video_coding::PacketBuffer packet_buffer_;
+  UniqueTimestampCounter frame_counter_ RTC_GUARDED_BY(worker_task_checker_);
+  SeqNumUnwrapper<uint16_t> frame_id_unwrapper_
+      RTC_GUARDED_BY(worker_task_checker_);
+
+  // Video structure provided in the dependency descriptor in a first packet
+  // of a key frame. It is required to parse dependency descriptor in the
+  // following delta packets.
+  std::unique_ptr<FrameDependencyStructure> video_structure_
+      RTC_GUARDED_BY(worker_task_checker_);
+  // Frame id of the last frame with the attached video structure.
+  // absl::nullopt when `video_structure_ == nullptr`;
+  absl::optional<int64_t> video_structure_frame_id_
+      RTC_GUARDED_BY(worker_task_checker_);
+
+  rtc::CriticalSection reference_finder_lock_;
+  std::unique_ptr<video_coding::RtpFrameReferenceFinder> reference_finder_
+      RTC_GUARDED_BY(reference_finder_lock_);
+  absl::optional<VideoCodecType> current_codec_;
+  uint32_t last_assembled_frame_rtp_timestamp_;
+
+  rtc::CriticalSection last_seq_num_cs_;
+  std::map<int64_t, uint16_t> last_seq_num_for_pic_id_
+      RTC_GUARDED_BY(last_seq_num_cs_);
+  video_coding::H264SpsPpsTracker tracker_;
+
+  // Maps payload id to the depacketizer.
+  std::map<uint8_t, std::unique_ptr<VideoRtpDepacketizer>> payload_type_map_;
+
+  // TODO(johan): Remove pt_codec_params_ once
+  // https://bugs.chromium.org/p/webrtc/issues/detail?id=6883 is resolved.
+  // Maps a payload type to a map of out-of-band supplied codec parameters.
+  std::map<uint8_t, std::map<std::string, std::string>> pt_codec_params_;
+  int16_t last_payload_type_ = -1;
+
+  bool has_received_frame_;
+
+  std::vector<RtpPacketSinkInterface*> secondary_sinks_
+      RTC_GUARDED_BY(worker_task_checker_);
+
+  // Info for GetSyncInfo is updated on network or worker thread, and queried on
+  // the worker thread.
+  rtc::CriticalSection sync_info_lock_;
+  absl::optional<uint32_t> last_received_rtp_timestamp_
+      RTC_GUARDED_BY(sync_info_lock_);
+  absl::optional<int64_t> last_received_rtp_system_time_ms_
+      RTC_GUARDED_BY(sync_info_lock_);
+
+  // Used to validate the buffered frame decryptor is always run on the correct
+  // thread.
+  rtc::ThreadChecker network_tc_;
+  // Handles incoming encrypted frames and forwards them to the
+  // rtp_reference_finder if they are decryptable.
+  std::unique_ptr<BufferedFrameDecryptor> buffered_frame_decryptor_
+      RTC_PT_GUARDED_BY(network_tc_);
+  std::atomic<bool> frames_decryptable_;
+  absl::optional<ColorSpace> last_color_space_;
+
+  AbsoluteCaptureTimeReceiver absolute_capture_time_receiver_
+      RTC_GUARDED_BY(worker_task_checker_);
+
+  int64_t last_completed_picture_id_ = 0;
+
+  rtc::scoped_refptr<RtpVideoStreamReceiverFrameTransformerDelegate>
+      frame_transformer_delegate_;
+};
+
+}  // namespace webrtc
+
+#endif  // VIDEO_RTP_VIDEO_STREAM_RECEIVER2_H_

video/rtp_video_stream_receiver_frame_transformer_delegate.cc

@@ -17,7 +17,6 @@
 #include "modules/rtp_rtcp/source/rtp_descriptor_authentication.h"
 #include "rtc_base/task_utils/to_queued_task.h"
 #include "rtc_base/thread.h"
-#include "video/rtp_video_stream_receiver.h"
 
 namespace webrtc {
 
@@ -69,7 +68,7 @@ class TransformableVideoReceiverFrame
 
 RtpVideoStreamReceiverFrameTransformerDelegate::
     RtpVideoStreamReceiverFrameTransformerDelegate(
-        RtpVideoStreamReceiver* receiver,
+        RtpVideoFrameReceiver* receiver,
         rtc::scoped_refptr<FrameTransformerInterface> frame_transformer,
         rtc::Thread* network_thread,
         uint32_t ssrc)

video/rtp_video_stream_receiver_frame_transformer_delegate.h

@@ -20,7 +20,16 @@
 
 namespace webrtc {
 
-class RtpVideoStreamReceiver;
+// Called back by RtpVideoStreamReceiverFrameTransformerDelegate on the network
+// thread after transformation.
+class RtpVideoFrameReceiver {
+ public:
+  virtual void ManageFrame(
+      std::unique_ptr<video_coding::RtpFrameObject> frame) = 0;
+
+ protected:
+  virtual ~RtpVideoFrameReceiver() = default;
+};
 
 // Delegates calls to FrameTransformerInterface to transform frames, and to
 // RtpVideoStreamReceiver to manage transformed frames on the |network_thread_|.
@@ -28,7 +37,7 @@ class RtpVideoStreamReceiverFrameTransformerDelegate
     : public TransformedFrameCallback {
  public:
   RtpVideoStreamReceiverFrameTransformerDelegate(
-      RtpVideoStreamReceiver* receiver,
+      RtpVideoFrameReceiver* receiver,
       rtc::scoped_refptr<FrameTransformerInterface> frame_transformer,
       rtc::Thread* network_thread,
       uint32_t ssrc);
@@ -44,7 +53,7 @@ class RtpVideoStreamReceiverFrameTransformerDelegate
   void OnTransformedFrame(
       std::unique_ptr<TransformableFrameInterface> frame) override;
 
-  // Delegates the call to RtpVideoReceiver::ManageFrame on the
+  // Delegates the call to RtpVideoFrameReceiver::ManageFrame on the
   // |network_thread_|.
   void ManageFrame(std::unique_ptr<TransformableFrameInterface> frame);
 
@@ -53,7 +62,7 @@ class RtpVideoStreamReceiverFrameTransformerDelegate
 
  private:
   SequenceChecker network_sequence_checker_;
-  RtpVideoStreamReceiver* receiver_ RTC_GUARDED_BY(network_sequence_checker_);
+  RtpVideoFrameReceiver* receiver_ RTC_GUARDED_BY(network_sequence_checker_);
   rtc::scoped_refptr<FrameTransformerInterface> frame_transformer_
       RTC_GUARDED_BY(network_sequence_checker_);
   rtc::Thread* const network_thread_;

video/rtp_video_stream_receiver_frame_transformer_delegate_unittest.cc

@@ -21,11 +21,11 @@
 #include "modules/rtp_rtcp/source/rtp_descriptor_authentication.h"
 #include "modules/utility/include/process_thread.h"
 #include "rtc_base/event.h"
+#include "rtc_base/ref_counted_object.h"
 #include "rtc_base/task_utils/to_queued_task.h"
 #include "test/gmock.h"
 #include "test/gtest.h"
 #include "test/mock_frame_transformer.h"
-#include "video/rtp_video_stream_receiver.h"
 
 namespace webrtc {
 namespace {
@@ -47,70 +47,10 @@ std::unique_ptr<video_coding::RtpFrameObject> CreateRtpFrameObject() {
   return CreateRtpFrameObject(RTPVideoHeader());
 }
 
-class FakeTransport : public Transport {
+class TestRtpVideoFrameReceiver : public RtpVideoFrameReceiver {
  public:
-  bool SendRtp(const uint8_t* packet,
+  TestRtpVideoFrameReceiver() {}
-               size_t length,
+  ~TestRtpVideoFrameReceiver() override = default;
-               const PacketOptions& options) {
-    return true;
-  }
-  bool SendRtcp(const uint8_t* packet, size_t length) { return true; }
-};
-
-class FakeNackSender : public NackSender {
- public:
-  void SendNack(const std::vector<uint16_t>& sequence_numbers) {}
-  void SendNack(const std::vector<uint16_t>& sequence_numbers,
-                bool buffering_allowed) {}
-};
-
-class FakeOnCompleteFrameCallback
-    : public video_coding::OnCompleteFrameCallback {
- public:
-  void OnCompleteFrame(
-      std::unique_ptr<video_coding::EncodedFrame> frame) override {}
-};
-
-class TestRtpVideoStreamReceiverInitializer {
- public:
-  TestRtpVideoStreamReceiverInitializer()
-      : test_config_(nullptr),
-        test_process_thread_(ProcessThread::Create("TestThread")) {
-    test_config_.rtp.remote_ssrc = 1111;
-    test_config_.rtp.local_ssrc = 2222;
-    test_rtp_receive_statistics_ =
-        ReceiveStatistics::Create(Clock::GetRealTimeClock());
-  }
-
- protected:
-  VideoReceiveStream::Config test_config_;
-  FakeTransport fake_transport_;
-  FakeNackSender fake_nack_sender_;
-  FakeOnCompleteFrameCallback fake_on_complete_frame_callback_;
-  std::unique_ptr<ProcessThread> test_process_thread_;
-  std::unique_ptr<ReceiveStatistics> test_rtp_receive_statistics_;
-};
-
-class TestRtpVideoStreamReceiver : public TestRtpVideoStreamReceiverInitializer,
-                                   public RtpVideoStreamReceiver {
- public:
-  TestRtpVideoStreamReceiver()
-      : TestRtpVideoStreamReceiverInitializer(),
-        RtpVideoStreamReceiver(Clock::GetRealTimeClock(),
-                               &fake_transport_,
-                               nullptr,
-                               nullptr,
-                               &test_config_,
-                               test_rtp_receive_statistics_.get(),
-                               nullptr,
-                               nullptr,
-                               test_process_thread_.get(),
-                               &fake_nack_sender_,
-                               nullptr,
-                               &fake_on_complete_frame_callback_,
-                               nullptr,
-                               nullptr) {}
-  ~TestRtpVideoStreamReceiver() override = default;
 
   MOCK_METHOD(void,
               ManageFrame,
@@ -120,7 +60,7 @@ class TestRtpVideoStreamReceiver : public TestRtpVideoStreamReceiverInitializer,
 
 TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest,
      RegisterTransformedFrameCallbackSinkOnInit) {
-  TestRtpVideoStreamReceiver receiver;
+  TestRtpVideoFrameReceiver receiver;
   rtc::scoped_refptr<MockFrameTransformer> frame_transformer(
       new rtc::RefCountedObject<MockFrameTransformer>());
   rtc::scoped_refptr<RtpVideoStreamReceiverFrameTransformerDelegate> delegate(
@@ -134,7 +74,7 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest,
 
 TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest,
      UnregisterTransformedFrameSinkCallbackOnReset) {
-  TestRtpVideoStreamReceiver receiver;
+  TestRtpVideoFrameReceiver receiver;
   rtc::scoped_refptr<MockFrameTransformer> frame_transformer(
       new rtc::RefCountedObject<MockFrameTransformer>());
   rtc::scoped_refptr<RtpVideoStreamReceiverFrameTransformerDelegate> delegate(
@@ -146,7 +86,7 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest,
 }
 
 TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest, TransformFrame) {
-  TestRtpVideoStreamReceiver receiver;
+  TestRtpVideoFrameReceiver receiver;
   rtc::scoped_refptr<MockFrameTransformer> frame_transformer(
       new rtc::RefCountedObject<testing::NiceMock<MockFrameTransformer>>());
   rtc::scoped_refptr<RtpVideoStreamReceiverFrameTransformerDelegate> delegate(
@@ -160,7 +100,7 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest, TransformFrame) {
 
 TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest,
      ManageFrameOnTransformedFrame) {
-  TestRtpVideoStreamReceiver receiver;
+  TestRtpVideoFrameReceiver receiver;
   rtc::scoped_refptr<MockFrameTransformer> mock_frame_transformer(
       new rtc::RefCountedObject<NiceMock<MockFrameTransformer>>());
   rtc::scoped_refptr<RtpVideoStreamReceiverFrameTransformerDelegate> delegate =
@@ -186,7 +126,7 @@ TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest,
 
 TEST(RtpVideoStreamReceiverFrameTransformerDelegateTest,
      TransformableFrameMetadataHasCorrectValue) {
-  TestRtpVideoStreamReceiver receiver;
+  TestRtpVideoFrameReceiver receiver;
   rtc::scoped_refptr<MockFrameTransformer> mock_frame_transformer =
       new rtc::RefCountedObject<NiceMock<MockFrameTransformer>>();
   rtc::scoped_refptr<RtpVideoStreamReceiverFrameTransformerDelegate> delegate =

video/video_receive_stream2.cc

@@ -49,7 +49,7 @@
 #include "system_wrappers/include/field_trial.h"
 #include "video/call_stats2.h"
 #include "video/frame_dumping_decoder.h"
-#include "video/receive_statistics_proxy.h"
+#include "video/receive_statistics_proxy2.h"
 
 namespace webrtc {
 

video/video_receive_stream2.h

@@ -30,7 +30,7 @@
 #include "system_wrappers/include/clock.h"
 #include "video/receive_statistics_proxy2.h"
 #include "video/rtp_streams_synchronizer2.h"
-#include "video/rtp_video_stream_receiver.h"
+#include "video/rtp_video_stream_receiver2.h"
 #include "video/transport_adapter.h"
 #include "video/video_stream_decoder2.h"
 
@@ -199,7 +199,7 @@ class VideoReceiveStream2 : public webrtc::VideoReceiveStream,
   std::unique_ptr<VCMTiming> timing_;  // Jitter buffer experiment.
   VideoReceiver2 video_receiver_;
   std::unique_ptr<rtc::VideoSinkInterface<VideoFrame>> incoming_video_stream_;
-  RtpVideoStreamReceiver rtp_video_stream_receiver_;
+  RtpVideoStreamReceiver2 rtp_video_stream_receiver_;
   std::unique_ptr<VideoStreamDecoder> video_stream_decoder_;
   RtpStreamsSynchronizer rtp_stream_sync_;