Split out counting unique rtp timestamps from packet_buffer

Bug: None
Change-Id: Ia6fd05f284e8304cf56ab9ddf944fb222a4c9573
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/158676
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29656}

modules/video_coding/BUILD.gn

@@ -130,6 +130,8 @@ rtc_library("video_coding") {
     "timestamp_map.h",
     "timing.cc",
     "timing.h",
+    "unique_timestamp_counter.cc",
+    "unique_timestamp_counter.h",
     "video_codec_initializer.cc",
     "video_receiver2.cc",
     "video_receiver2.h",
@@ -849,6 +851,7 @@ if (rtc_include_tests) {
       "test/stream_generator.cc",
       "test/stream_generator.h",
       "timing_unittest.cc",
+      "unique_timestamp_counter_unittest.cc",
       "utility/decoded_frames_history_unittest.cc",
       "utility/default_video_bitrate_allocator_unittest.cc",
       "utility/frame_dropper_unittest.cc",

modules/video_coding/packet_buffer.cc

@@ -40,7 +40,6 @@ PacketBuffer::PacketBuffer(Clock* clock,
       first_packet_received_(false),
       is_cleared_to_first_seq_num_(false),
       buffer_(start_buffer_size),
-      unique_frames_seen_(0),
       sps_pps_idr_is_h264_keyframe_(
           field_trial::IsEnabled("WebRTC-SpsPpsIdrIsH264Keyframe")) {
   RTC_DCHECK_LE(start_buffer_size, max_buffer_size);
@@ -56,7 +55,6 @@ PacketBuffer::~PacketBuffer() {
 PacketBuffer::InsertResult PacketBuffer::InsertPacket(VCMPacket* packet) {
   PacketBuffer::InsertResult result;
   rtc::CritScope lock(&crit_);
-  OnTimestampReceived(packet->timestamp);
 
   uint16_t seq_num = packet->seqNum;
   size_t index = seq_num % buffer_.size();
@@ -208,11 +206,6 @@ absl::optional<int64_t> PacketBuffer::LastReceivedKeyframePacketMs() const {
   return last_received_keyframe_packet_ms_;
 }
 
-int PacketBuffer::GetUniqueFramesSeen() const {
-  rtc::CritScope lock(&crit_);
-  return unique_frames_seen_;
-}
-
 bool PacketBuffer::ExpandBufferSize() {
   if (buffer_.size() == max_size_) {
     RTC_LOG(LS_WARNING) << "PacketBuffer is already at max size (" << max_size_
@@ -486,18 +479,5 @@ void PacketBuffer::UpdateMissingPackets(uint16_t seq_num) {
   }
 }
 
-void PacketBuffer::OnTimestampReceived(uint32_t rtp_timestamp) {
-  const size_t kMaxTimestampsHistory = 1000;
-  if (rtp_timestamps_history_set_.insert(rtp_timestamp).second) {
-    rtp_timestamps_history_queue_.push(rtp_timestamp);
-    ++unique_frames_seen_;
-    if (rtp_timestamps_history_set_.size() > kMaxTimestampsHistory) {
-      uint32_t discarded_timestamp = rtp_timestamps_history_queue_.front();
-      rtp_timestamps_history_set_.erase(discarded_timestamp);
-      rtp_timestamps_history_queue_.pop();
-    }
-  }
-}
-
 }  // namespace video_coding
 }  // namespace webrtc

modules/video_coding/packet_buffer.h

@@ -52,9 +52,6 @@ class PacketBuffer {
   absl::optional<int64_t> LastReceivedPacketMs() const;
   absl::optional<int64_t> LastReceivedKeyframePacketMs() const;
 
-  // Returns number of different frames seen in the packet buffer
-  int GetUniqueFramesSeen() const;
-
  private:
   struct StoredPacket {
     uint16_t seq_num() const { return data.seqNum; }
@@ -104,10 +101,6 @@ class PacketBuffer {
   void UpdateMissingPackets(uint16_t seq_num)
       RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_);
 
-  // Counts unique received timestamps and updates |unique_frames_seen_|.
-  void OnTimestampReceived(uint32_t rtp_timestamp)
-      RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_);
-
   rtc::CriticalSection crit_;
 
   // buffer_.size() and max_size_ must always be a power of two.
@@ -131,8 +124,6 @@ class PacketBuffer {
   absl::optional<int64_t> last_received_keyframe_packet_ms_
       RTC_GUARDED_BY(crit_);
 
-  int unique_frames_seen_ RTC_GUARDED_BY(crit_);
-
   absl::optional<uint16_t> newest_inserted_seq_num_ RTC_GUARDED_BY(crit_);
   std::set<uint16_t, DescendingSeqNumComp<uint16_t>> missing_packets_
       RTC_GUARDED_BY(crit_);
@@ -140,11 +131,6 @@ class PacketBuffer {
   // Indicates if we should require SPS, PPS, and IDR for a particular
   // RTP timestamp to treat the corresponding frame as a keyframe.
   const bool sps_pps_idr_is_h264_keyframe_;
-
-  // Stores several last seen unique timestamps for quick search.
-  std::set<uint32_t> rtp_timestamps_history_set_ RTC_GUARDED_BY(crit_);
-  // Stores the same unique timestamps in the order of insertion.
-  std::queue<uint32_t> rtp_timestamps_history_queue_ RTC_GUARDED_BY(crit_);
 };
 
 }  // namespace video_coding

modules/video_coding/packet_buffer_unittest.cc

@@ -225,57 +225,6 @@ TEST_F(PacketBufferTest, FrameSize) {
               ElementsAre(Pointee(SizeIs(20))));
 }
 
-TEST_F(PacketBufferTest, CountsUniqueFrames) {
-  const uint16_t seq_num = Rand();
-
-  ASSERT_EQ(packet_buffer_.GetUniqueFramesSeen(), 0);
-
-  Insert(seq_num, kKeyFrame, kFirst, kNotLast, 0, nullptr, 100);
-  ASSERT_EQ(packet_buffer_.GetUniqueFramesSeen(), 1);
-  // Still the same frame.
-  Insert(seq_num + 1, kKeyFrame, kNotFirst, kLast, 0, nullptr, 100);
-  ASSERT_EQ(packet_buffer_.GetUniqueFramesSeen(), 1);
-
-  // Second frame.
-  Insert(seq_num + 2, kKeyFrame, kFirst, kNotLast, 0, nullptr, 200);
-  ASSERT_EQ(packet_buffer_.GetUniqueFramesSeen(), 2);
-  Insert(seq_num + 3, kKeyFrame, kNotFirst, kLast, 0, nullptr, 200);
-  ASSERT_EQ(packet_buffer_.GetUniqueFramesSeen(), 2);
-
-  // Old packet.
-  Insert(seq_num + 1, kKeyFrame, kNotFirst, kLast, 0, nullptr, 100);
-  ASSERT_EQ(packet_buffer_.GetUniqueFramesSeen(), 2);
-
-  // Missing middle packet.
-  Insert(seq_num + 4, kKeyFrame, kFirst, kNotLast, 0, nullptr, 300);
-  Insert(seq_num + 6, kKeyFrame, kNotFirst, kLast, 0, nullptr, 300);
-  ASSERT_EQ(packet_buffer_.GetUniqueFramesSeen(), 3);
-}
-
-TEST_F(PacketBufferTest, HasHistoryOfUniqueFrames) {
-  const int kNumFrames = 1500;
-  const int kRequiredHistoryLength = 1000;
-  const uint16_t seq_num = Rand();
-  const uint32_t timestamp = 0xFFFFFFF0;  // Large enough to cause wrap-around.
-
-  for (int i = 0; i < kNumFrames; ++i) {
-    Insert(seq_num + i, kKeyFrame, kFirst, kNotLast, 0, nullptr,
-           timestamp + 10 * i);
-  }
-  EXPECT_EQ(packet_buffer_.GetUniqueFramesSeen(), kNumFrames);
-
-  // Old packets within history should not affect number of seen unique frames.
-  for (int i = kNumFrames - kRequiredHistoryLength; i < kNumFrames; ++i) {
-    Insert(seq_num + i, kKeyFrame, kFirst, kNotLast, 0, nullptr,
-           timestamp + 10 * i);
-  }
-  EXPECT_EQ(packet_buffer_.GetUniqueFramesSeen(), kNumFrames);
-
-  // Very old packets should be treated as unique.
-  Insert(seq_num, kKeyFrame, kFirst, kNotLast, 0, nullptr, timestamp);
-  EXPECT_EQ(packet_buffer_.GetUniqueFramesSeen(), kNumFrames + 1);
-}
-
 TEST_F(PacketBufferTest, ExpandBuffer) {
   const uint16_t seq_num = Rand();
 

modules/video_coding/unique_timestamp_counter.cc

@@ -0,0 +1,41 @@
+/*
+ *  Copyright (c) 2019 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/unique_timestamp_counter.h"
+
+#include <cstdint>
+#include <memory>
+#include <set>
+
+namespace webrtc {
+namespace {
+
+constexpr int kMaxHistory = 1000;
+
+}  // namespace
+
+UniqueTimestampCounter::UniqueTimestampCounter()
+    : latest_(std::make_unique<uint32_t[]>(kMaxHistory)) {}
+
+void UniqueTimestampCounter::Add(uint32_t value) {
+  if (value == last_ || !search_index_.insert(value).second) {
+    // Already known.
+    return;
+  }
+  int index = unique_seen_ % kMaxHistory;
+  if (unique_seen_ >= kMaxHistory) {
+    search_index_.erase(latest_[index]);
+  }
+  latest_[index] = value;
+  last_ = value;
+  ++unique_seen_;
+}
+
+}  // namespace webrtc

modules/video_coding/unique_timestamp_counter.h

@@ -0,0 +1,44 @@
+/*
+ *  Copyright (c) 2019 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_UNIQUE_TIMESTAMP_COUNTER_H_
+#define MODULES_VIDEO_CODING_UNIQUE_TIMESTAMP_COUNTER_H_
+
+#include <cstdint>
+#include <memory>
+#include <set>
+
+namespace webrtc {
+
+// Counts number of uniquly seen frames (aka pictures, aka temporal units)
+// identified by their rtp timestamp.
+class UniqueTimestampCounter {
+ public:
+  UniqueTimestampCounter();
+  UniqueTimestampCounter(const UniqueTimestampCounter&) = delete;
+  UniqueTimestampCounter& operator=(const UniqueTimestampCounter&) = delete;
+  ~UniqueTimestampCounter() = default;
+
+  void Add(uint32_t timestamp);
+  // Returns number of different |timestamp| passed to the UniqueCounter.
+  int GetUniqueSeen() const { return unique_seen_; }
+
+ private:
+  int unique_seen_ = 0;
+  // Stores several last seen unique values for quick search.
+  std::set<uint32_t> search_index_;
+  // The same unique values in the circular buffer in the insertion order.
+  std::unique_ptr<uint32_t[]> latest_;
+  // Last inserted value for optimization purpose.
+  int64_t last_ = -1;
+};
+
+}  // namespace webrtc
+
+#endif  // MODULES_VIDEO_CODING_UNIQUE_TIMESTAMP_COUNTER_H_

modules/video_coding/unique_timestamp_counter_unittest.cc

@@ -0,0 +1,52 @@
+/*
+ *  Copyright (c) 2019 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/unique_timestamp_counter.h"
+
+#include "test/gtest.h"
+
+namespace webrtc {
+namespace {
+
+TEST(UniqueTimestampCounterTest, InitiallyZero) {
+  UniqueTimestampCounter counter;
+  EXPECT_EQ(counter.GetUniqueSeen(), 0);
+}
+
+TEST(UniqueTimestampCounterTest, CountsUniqueValues) {
+  UniqueTimestampCounter counter;
+  counter.Add(100);
+  counter.Add(100);
+  counter.Add(200);
+  counter.Add(150);
+  counter.Add(100);
+  EXPECT_EQ(counter.GetUniqueSeen(), 3);
+}
+
+TEST(UniqueTimestampCounterTest, ForgetsOldValuesAfter1000NewValues) {
+  const int kNumValues = 1500;
+  const int kMaxHistory = 1000;
+  const uint32_t value = 0xFFFFFFF0;
+  UniqueTimestampCounter counter;
+  for (int i = 0; i < kNumValues; ++i) {
+    counter.Add(value + 10 * i);
+  }
+  ASSERT_EQ(counter.GetUniqueSeen(), kNumValues);
+  // Slightly old values not affect number of seen unique values.
+  for (int i = kNumValues - kMaxHistory; i < kNumValues; ++i) {
+    counter.Add(value + 10 * i);
+  }
+  EXPECT_EQ(counter.GetUniqueSeen(), kNumValues);
+  // Very old value will be treated as unique.
+  counter.Add(value);
+  EXPECT_EQ(counter.GetUniqueSeen(), kNumValues + 1);
+}
+
+}  // namespace
+}  // namespace webrtc

video/rtp_video_stream_receiver.cc

@@ -326,6 +326,7 @@ void RtpVideoStreamReceiver::OnReceivedPayloadData(
     rtc::ArrayView<const uint8_t> codec_payload,
     const RtpPacketReceived& rtp_packet,
     const RTPVideoHeader& video) {
+  RTC_DCHECK_RUN_ON(&worker_task_checker_);
   RTPHeader rtp_header;
   rtp_packet.GetHeader(&rtp_header);
   VCMPacket packet(codec_payload.data(), codec_payload.size(), rtp_header,
@@ -466,6 +467,7 @@ void RtpVideoStreamReceiver::OnReceivedPayloadData(
   }
 
   rtcp_feedback_buffer_.SendBufferedRtcpFeedback();
+  frame_counter_.Add(packet.timestamp);
   OnInsertedPacket(packet_buffer_.InsertPacket(&packet));
 }
 
@@ -877,10 +879,6 @@ void RtpVideoStreamReceiver::SignalNetworkState(NetworkState state) {
                                                : RtcpMode::kOff);
 }
 
-int RtpVideoStreamReceiver::GetUniqueFramesSeen() const {
-  return packet_buffer_.GetUniqueFramesSeen();
-}
-
 void RtpVideoStreamReceiver::StartReceive() {
   RTC_DCHECK_RUN_ON(&worker_task_checker_);
   receiving_ = true;

video/rtp_video_stream_receiver.h

@@ -37,6 +37,7 @@
 #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/numerics/sequence_number_util.h"
@@ -103,8 +104,11 @@ class RtpVideoStreamReceiver : public LossNotificationSender,
 
   void SignalNetworkState(NetworkState state);
 
-  // Returns number of different frames seen in the packet buffer.
-  int GetUniqueFramesSeen() const;
+  // 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;
@@ -270,6 +274,7 @@ class RtpVideoStreamReceiver : public LossNotificationSender,
   std::unique_ptr<LossNotificationController> loss_notification_controller_;
 
   video_coding::PacketBuffer packet_buffer_;
+  UniqueTimestampCounter frame_counter_ RTC_GUARDED_BY(worker_task_checker_);
 
   rtc::CriticalSection reference_finder_lock_;
   std::unique_ptr<video_coding::RtpFrameReferenceFinder> reference_finder_