From eb648bf0e5a9bae185bcd6b4b3be371e1da3507d Mon Sep 17 00:00:00 2001 From: kjellander Date: Mon, 7 Mar 2016 09:56:26 -0800 Subject: [PATCH] Revert of Implement the NackModule as part of the new jitter buffer. (patchset #19 id:360001 of https://codereview.webrtc.org/1715673002/ ) Reason for revert: Unfortunately this breaks in the main waterfall: https://build.chromium.org/p/client.webrtc/builders/Android32%20Builder/builds/6362 I think it's related to dcheck_always_on=1 which is set in GYP_DEFINES only on the trybots, but not on the bots in the main waterfall. Original issue's description: > Implement the NackModule as part of the new jitter buffer. > > Things done/implemented in this CL: > - An interface that can send Nack (VCMNackSender). > - An interface that can request KeyFrames (VCMKeyFrameRequestSender). > - The nack module (NackModule). > - A set of convenience functions for modular numbers (mod_ops.h). > > BUG=webrtc:5514 > > Committed: https://crrev.com/f472c5b6722dfb221f929fc4d3a2b4ca54647701 > Cr-Commit-Position: refs/heads/master@{#11882} TBR=sprang@webrtc.org,stefan@webrtc.org,terelius@webrtc.org,torbjorng@webrtc.org,perkj@webrtc.org,tommi@webrtc.org,philipel@webrtc.org BUG=webrtc:5514 NOTRY=True Review URL: https://codereview.webrtc.org/1771883002 Cr-Commit-Position: refs/heads/master@{#11887} --- webrtc/base/base.gyp | 1 - webrtc/base/base_tests.gyp | 1 - webrtc/base/mod_ops.h | 123 -------- webrtc/base/mod_ops_unittest.cc | 172 ----------- webrtc/modules/modules.gyp | 2 - webrtc/modules/video_coding/histogram.cc | 62 ---- webrtc/modules/video_coding/histogram.h | 46 --- .../video_coding/histogram_unittest.cc | 76 ----- .../include/video_coding_defines.h | 20 -- webrtc/modules/video_coding/nack_module.cc | 257 --------------- webrtc/modules/video_coding/nack_module.h | 101 ------ .../video_coding/nack_module_unittest.cc | 292 ------------------ webrtc/modules/video_coding/video_coding.gypi | 4 - 13 files changed, 1157 deletions(-) delete mode 100644 webrtc/base/mod_ops.h delete mode 100644 webrtc/base/mod_ops_unittest.cc delete mode 100644 webrtc/modules/video_coding/histogram.cc delete mode 100644 webrtc/modules/video_coding/histogram.h delete mode 100644 webrtc/modules/video_coding/histogram_unittest.cc delete mode 100644 webrtc/modules/video_coding/nack_module.cc delete mode 100644 webrtc/modules/video_coding/nack_module.h delete mode 100644 webrtc/modules/video_coding/nack_module_unittest.cc diff --git a/webrtc/base/base.gyp b/webrtc/base/base.gyp index 5ac173ca91..a6b2b1cd4a 100644 --- a/webrtc/base/base.gyp +++ b/webrtc/base/base.gyp @@ -102,7 +102,6 @@ 'md5.h', 'md5digest.cc', 'md5digest.h', - 'mod_ops.h', 'optional.h', 'platform_file.cc', 'platform_file.h', diff --git a/webrtc/base/base_tests.gyp b/webrtc/base/base_tests.gyp index 2467b7471a..566ddeb44e 100644 --- a/webrtc/base/base_tests.gyp +++ b/webrtc/base/base_tests.gyp @@ -75,7 +75,6 @@ 'md5digest_unittest.cc', 'messagedigest_unittest.cc', 'messagequeue_unittest.cc', - 'mod_ops_unittest.cc', 'multipart_unittest.cc', 'nat_unittest.cc', 'network_unittest.cc', diff --git a/webrtc/base/mod_ops.h b/webrtc/base/mod_ops.h deleted file mode 100644 index 9e649b75d2..0000000000 --- a/webrtc/base/mod_ops.h +++ /dev/null @@ -1,123 +0,0 @@ -/* - * 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 WEBRTC_BASE_MOD_OPS_H_ -#define WEBRTC_BASE_MOD_OPS_H_ - -#include -#include - -#include "webrtc/base/checks.h" - -#define MOD_OPS_ASSERT_TYPE_IS_UNSIGNED(T) \ - static_assert(std::numeric_limits::is_integer && \ - !std::numeric_limits::is_signed, \ - "Type must be of unsigned integer.") - -namespace webrtc { - -template // NOLINT -inline unsigned long Add(unsigned long a, unsigned long b) { // NOLINT - RTC_DCHECK_LT(a, M); - unsigned long t = M - b % M; // NOLINT - unsigned long res = a - t; // NOLINT - if (t > a) - return res + M; - return res; -} - -template // NOLINT -inline unsigned long Subtract(unsigned long a, unsigned long b) { // NOLINT - RTC_DCHECK_LT(a, M); - unsigned long sub = b % M; // NOLINT - if (a < sub) - return M - (sub - a); - return a - sub; -} - -// Calculates the forward difference between two numbers. -// -// Example: -// uint8_t x = 253; -// uint8_t y = 2; -// -// ForwardDiff(x, y) == 4 -// -// 252 253 254 255 0 1 2 3 -// ################################################# -// | | x | | | | | y | | -// ################################################# -// |----->----->----->----->-----> -// -// ForwardDiff(y, x) == 251 -// -// 252 253 254 255 0 1 2 3 -// ################################################# -// | | x | | | | | y | | -// ################################################# -// -->-----> |----->--- -// -template -inline T ForwardDiff(T a, T b) { - MOD_OPS_ASSERT_TYPE_IS_UNSIGNED(T); - return b - a; -} - -// Calculates the reverse difference between two numbers. -// -// Example: -// uint8_t x = 253; -// uint8_t y = 2; -// -// ReverseDiff(y, x) == 5 -// -// 252 253 254 255 0 1 2 3 -// ################################################# -// | | x | | | | | y | | -// ################################################# -// <-----<-----<-----<-----<-----| -// -// ReverseDiff(x, y) == 251 -// -// 252 253 254 255 0 1 2 3 -// ################################################# -// | | x | | | | | y | | -// ################################################# -// ---<-----| |<-----<-- -// -template -inline T ReverseDiff(T a, T b) { - MOD_OPS_ASSERT_TYPE_IS_UNSIGNED(T); - return a - b; -} - -// Test if the sequence number a is ahead or at sequence number b. -// If the two sequence numbers are at max distance from each other -// then the sequence number with highest value is considered to -// be ahead. -template -inline bool AheadOrAt(T a, T b) { - MOD_OPS_ASSERT_TYPE_IS_UNSIGNED(T); - const T maxDist = std::numeric_limits::max() / 2 + T(1); - if (a - b == maxDist) - return b < a; - return ForwardDiff(b, a) < maxDist; -} - -// Test if sequence number a is ahead of sequence number b. -template -inline bool AheadOf(T a, T b) { - MOD_OPS_ASSERT_TYPE_IS_UNSIGNED(T); - return a != b && AheadOrAt(a, b); -} - -} // namespace webrtc - -#endif // WEBRTC_BASE_MOD_OPS_H_ diff --git a/webrtc/base/mod_ops_unittest.cc b/webrtc/base/mod_ops_unittest.cc deleted file mode 100644 index 13e638fb6e..0000000000 --- a/webrtc/base/mod_ops_unittest.cc +++ /dev/null @@ -1,172 +0,0 @@ -/* - * 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 "testing/gtest/include/gtest/gtest.h" -#include "webrtc/base/mod_ops.h" - -namespace webrtc { -class TestModOps : public ::testing::Test { - protected: - // Can't use std::numeric_limits::max() since - // MSVC doesn't support constexpr. - static const unsigned long ulmax = ~0ul; // NOLINT -}; - -TEST_F(TestModOps, Add) { - const int D = 100; - EXPECT_EQ(1u, Add(0, 1)); - EXPECT_EQ(0u, Add(0, D)); - for (int i = 0; i < D; ++i) - EXPECT_EQ(0u, Add(i, D - i)); - - int t = 37; - uint8_t a = t; - for (int i = 0; i < 256; ++i) { - EXPECT_EQ(a, static_cast(t)); - t = Add<256>(t, 1); - ++a; - } -} - -TEST_F(TestModOps, AddLarge) { - // NOLINTNEXTLINE - const unsigned long D = ulmax - 10ul; // NOLINT - unsigned long l = D - 1ul; // NOLINT - EXPECT_EQ(D - 2ul, Add(l, l)); - EXPECT_EQ(9ul, Add(l, ulmax)); - EXPECT_EQ(10ul, Add(0ul, ulmax)); -} - -TEST_F(TestModOps, Subtract) { - const int D = 100; - EXPECT_EQ(99u, Subtract(0, 1)); - EXPECT_EQ(0u, Subtract(0, D)); - for (int i = 0; i < D; ++i) - EXPECT_EQ(0u, Subtract(i, D + i)); - - int t = 37; - uint8_t a = t; - for (int i = 0; i < 256; ++i) { - EXPECT_EQ(a, static_cast(t)); - t = Subtract<256>(t, 1); - --a; - } -} - -TEST_F(TestModOps, SubtractLarge) { - // NOLINTNEXTLINE - const unsigned long D = ulmax - 10ul; // NOLINT - unsigned long l = D - 1ul; // NOLINT - EXPECT_EQ(0ul, Subtract(l, l)); - EXPECT_EQ(D - 11ul, Subtract(l, ulmax)); - EXPECT_EQ(D - 10ul, Subtract(0ul, ulmax)); -} - -TEST_F(TestModOps, ForwardDiff) { - EXPECT_EQ(0u, ForwardDiff(4711u, 4711u)); - - uint8_t x = 0; - uint8_t y = 255; - for (int i = 0; i < 256; ++i) { - EXPECT_EQ(255u, ForwardDiff(x, y)); - ++x; - ++y; - } - - int yi = 255; - for (int i = 0; i < 256; ++i) { - EXPECT_EQ(255u, ForwardDiff(x, yi)); - ++x; - ++yi; - } -} - -TEST_F(TestModOps, ReverseDiff) { - EXPECT_EQ(0u, ReverseDiff(4711u, 4711u)); - - uint8_t x = 0; - uint8_t y = 255; - for (int i = 0; i < 256; ++i) { - EXPECT_EQ(1u, ReverseDiff(x, y)); - ++x; - ++y; - } - - int yi = 255; - for (int i = 0; i < 256; ++i) { - EXPECT_EQ(1u, ReverseDiff(x, yi)); - ++x; - ++yi; - } -} - -TEST_F(TestModOps, AheadOrAt) { - uint8_t x = 0; - uint8_t y = 0; - EXPECT_TRUE(AheadOrAt(x, y)); - ++x; - EXPECT_TRUE(AheadOrAt(x, y)); - EXPECT_FALSE(AheadOrAt(y, x)); - for (int i = 0; i < 256; ++i) { - EXPECT_TRUE(AheadOrAt(x, y)); - ++x; - ++y; - } - - x = 128; - y = 0; - EXPECT_TRUE(AheadOrAt(x, y)); - EXPECT_FALSE(AheadOrAt(y, x)); - - x = 129; - EXPECT_FALSE(AheadOrAt(x, y)); - EXPECT_TRUE(AheadOrAt(y, x)); - EXPECT_TRUE(AheadOrAt(x, y)); - EXPECT_FALSE(AheadOrAt(y, x)); -} - -TEST_F(TestModOps, AheadOf) { - uint8_t x = 0; - uint8_t y = 0; - EXPECT_FALSE(AheadOf(x, y)); - ++x; - EXPECT_TRUE(AheadOf(x, y)); - EXPECT_FALSE(AheadOf(y, x)); - for (int i = 0; i < 256; ++i) { - EXPECT_TRUE(AheadOf(x, y)); - ++x; - ++y; - } - - x = 128; - y = 0; - for (int i = 0; i < 128; ++i) { - EXPECT_TRUE(AheadOf(x, y)); - EXPECT_FALSE(AheadOf(y, x)); - x++; - y++; - } - - for (int i = 0; i < 128; ++i) { - EXPECT_FALSE(AheadOf(x, y)); - EXPECT_TRUE(AheadOf(y, x)); - x++; - y++; - } - - x = 129; - y = 0; - EXPECT_FALSE(AheadOf(x, y)); - EXPECT_TRUE(AheadOf(y, x)); - EXPECT_TRUE(AheadOf(x, y)); - EXPECT_FALSE(AheadOf(y, x)); -} - -} // namespace webrtc diff --git a/webrtc/modules/modules.gyp b/webrtc/modules/modules.gyp index 5deef3596c..2f88cfc988 100644 --- a/webrtc/modules/modules.gyp +++ b/webrtc/modules/modules.gyp @@ -363,11 +363,9 @@ 'video_coding/include/mock/mock_vcm_callbacks.h', 'video_coding/bitrate_adjuster_unittest.cc', 'video_coding/decoding_state_unittest.cc', - 'video_coding/histogram_unittest.cc', 'video_coding/jitter_buffer_unittest.cc', 'video_coding/jitter_estimator_tests.cc', 'video_coding/media_optimization_unittest.cc', - 'video_coding/nack_module_unittest.cc', 'video_coding/receiver_unittest.cc', 'video_coding/session_info_unittest.cc', 'video_coding/timing_unittest.cc', diff --git a/webrtc/modules/video_coding/histogram.cc b/webrtc/modules/video_coding/histogram.cc deleted file mode 100644 index 749595b163..0000000000 --- a/webrtc/modules/video_coding/histogram.cc +++ /dev/null @@ -1,62 +0,0 @@ -/* - * 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 "webrtc/modules/video_coding/histogram.h" - -#include - -#include "webrtc/base/mod_ops.h" - -namespace webrtc { -namespace video_coding { -Histogram::Histogram(size_t num_buckets, size_t max_num_values) { - RTC_DCHECK_LE(0u, num_buckets); - RTC_DCHECK_LE(0u, max_num_values); - buckets_.resize(num_buckets); - values_.reserve(max_num_values); - index_ = 0; -} - -void Histogram::Add(size_t value) { - RTC_DCHECK_GE(value, 0u); - value = std::min(value, buckets_.size() - 1); - if (index_ < values_.size()) { - --buckets_[values_[index_]]; - RTC_DCHECK_LT(values_[index_], buckets_.size()); - values_[index_] = value; - } else { - values_.emplace_back(value); - } - - ++buckets_[value]; - index_ = (index_ + 1) % values_.capacity(); -} - -size_t Histogram::InverseCdf(float probability) const { - RTC_DCHECK_GE(probability, 0.f); - RTC_DCHECK_LE(probability, 1.f); - RTC_DCHECK_GT(values_.size(), 0ul); - - size_t bucket = 0; - float accumulated_probability = 0; - while (accumulated_probability < probability && bucket < buckets_.size()) { - accumulated_probability += - static_cast(buckets_[bucket]) / values_.size(); - ++bucket; - } - return bucket; -} - -size_t Histogram::NumValues() const { - return values_.size(); -} - -} // namespace video_coding -} // namespace webrtc diff --git a/webrtc/modules/video_coding/histogram.h b/webrtc/modules/video_coding/histogram.h deleted file mode 100644 index 7a99a1c012..0000000000 --- a/webrtc/modules/video_coding/histogram.h +++ /dev/null @@ -1,46 +0,0 @@ -/* - * 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 WEBRTC_MODULES_VIDEO_CODING_HISTOGRAM_H_ -#define WEBRTC_MODULES_VIDEO_CODING_HISTOGRAM_H_ - -#include -#include - -namespace webrtc { -namespace video_coding { -class Histogram { - public: - // A discrete histogram where every bucket with range [0, num_buckets). - // Values greater or equal to num_buckets will be placed in the last bucket. - Histogram(size_t num_buckets, size_t max_num_values); - - // Add a value to the histogram. If there already is max_num_values in the - // histogram then the oldest value will be replaced with the new value. - void Add(size_t value); - - // Calculates how many buckets have to be summed in order to accumulate at - // least the given probability. - size_t InverseCdf(float probability) const; - - // How many values that make up this histogram. - size_t NumValues() const; - - private: - // A circular buffer that holds the values that make up the histogram. - std::vector values_; - std::vector buckets_; - size_t index_; -}; - -} // namespace video_coding -} // namespace webrtc - -#endif // WEBRTC_MODULES_VIDEO_CODING_HISTOGRAM_H_ diff --git a/webrtc/modules/video_coding/histogram_unittest.cc b/webrtc/modules/video_coding/histogram_unittest.cc deleted file mode 100644 index ac79ac4afa..0000000000 --- a/webrtc/modules/video_coding/histogram_unittest.cc +++ /dev/null @@ -1,76 +0,0 @@ -/* - * 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 "testing/gtest/include/gtest/gtest.h" -#include "webrtc/modules/video_coding/histogram.h" - -namespace webrtc { -namespace video_coding { - -class TestHistogram : public ::testing::Test { - protected: - TestHistogram() : histogram_(5, 10) {} - Histogram histogram_; -}; - -TEST_F(TestHistogram, NumValues) { - EXPECT_EQ(0ul, histogram_.NumValues()); - histogram_.Add(0); - EXPECT_EQ(1ul, histogram_.NumValues()); -} - -TEST_F(TestHistogram, InverseCdf) { - histogram_.Add(0); - histogram_.Add(1); - histogram_.Add(2); - histogram_.Add(3); - histogram_.Add(4); - EXPECT_EQ(5ul, histogram_.NumValues()); - EXPECT_EQ(1ul, histogram_.InverseCdf(0.2f)); - EXPECT_EQ(2ul, histogram_.InverseCdf(0.2000001f)); - EXPECT_EQ(4ul, histogram_.InverseCdf(0.8f)); - - histogram_.Add(0); - EXPECT_EQ(6ul, histogram_.NumValues()); - EXPECT_EQ(1ul, histogram_.InverseCdf(0.2f)); - EXPECT_EQ(1ul, histogram_.InverseCdf(0.2000001f)); -} - -TEST_F(TestHistogram, ReplaceOldValues) { - histogram_.Add(0); - histogram_.Add(0); - histogram_.Add(0); - histogram_.Add(0); - histogram_.Add(0); - histogram_.Add(1); - histogram_.Add(1); - histogram_.Add(1); - histogram_.Add(1); - histogram_.Add(1); - EXPECT_EQ(10ul, histogram_.NumValues()); - EXPECT_EQ(1ul, histogram_.InverseCdf(0.5f)); - EXPECT_EQ(2ul, histogram_.InverseCdf(0.5000001f)); - - histogram_.Add(4); - histogram_.Add(4); - histogram_.Add(4); - histogram_.Add(4); - EXPECT_EQ(10ul, histogram_.NumValues()); - EXPECT_EQ(1ul, histogram_.InverseCdf(0.1f)); - EXPECT_EQ(2ul, histogram_.InverseCdf(0.5f)); - - histogram_.Add(20); - EXPECT_EQ(10ul, histogram_.NumValues()); - EXPECT_EQ(2ul, histogram_.InverseCdf(0.5f)); - EXPECT_EQ(5ul, histogram_.InverseCdf(0.5000001f)); -} - -} // namespace video_coding -} // namespace webrtc diff --git a/webrtc/modules/video_coding/include/video_coding_defines.h b/webrtc/modules/video_coding/include/video_coding_defines.h index 4fe8c79793..7d084c815b 100644 --- a/webrtc/modules/video_coding/include/video_coding_defines.h +++ b/webrtc/modules/video_coding/include/video_coding_defines.h @@ -11,8 +11,6 @@ #ifndef WEBRTC_MODULES_VIDEO_CODING_INCLUDE_VIDEO_CODING_DEFINES_H_ #define WEBRTC_MODULES_VIDEO_CODING_INCLUDE_VIDEO_CODING_DEFINES_H_ -#include - #include "webrtc/modules/include/module_common_types.h" #include "webrtc/typedefs.h" #include "webrtc/video_frame.h" @@ -162,8 +160,6 @@ class VCMFrameTypeCallback { // Callback class used for telling the user about which packet sequence numbers // are currently // missing and need to be resent. -// TODO(philipel): Deprecate VCMPacketRequestCallback -// and use NackSender instead. class VCMPacketRequestCallback { public: virtual int32_t ResendPackets(const uint16_t* sequenceNumbers, @@ -173,22 +169,6 @@ class VCMPacketRequestCallback { virtual ~VCMPacketRequestCallback() {} }; -class NackSender { - public: - virtual void SendNack(const std::vector& sequence_numbers) = 0; - - protected: - virtual ~NackSender() {} -}; - -class KeyFrameRequestSender { - public: - virtual void RequestKeyFrame() = 0; - - protected: - virtual ~KeyFrameRequestSender() {} -}; - // Callback used to inform the user of the the desired resolution // as subscribed by Media Optimization (Quality Modes) class VCMQMSettingsCallback { diff --git a/webrtc/modules/video_coding/nack_module.cc b/webrtc/modules/video_coding/nack_module.cc deleted file mode 100644 index 319a902ca2..0000000000 --- a/webrtc/modules/video_coding/nack_module.cc +++ /dev/null @@ -1,257 +0,0 @@ -/* - * 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 -#include - -#include "webrtc/modules/video_coding/nack_module.h" - -#include "webrtc/base/checks.h" -#include "webrtc/base/logging.h" -#include "webrtc/base/mod_ops.h" -#include "webrtc/modules/utility/include/process_thread.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; -} // namespace - -NackModule::NackInfo::NackInfo() - : seq_num(0), send_at_seq_num(0), sent_at_time(-1), retries(0) {} - -NackModule::NackInfo::NackInfo(uint16_t seq_num, uint16_t send_at_seq_num) - : seq_num(seq_num), - send_at_seq_num(send_at_seq_num), - sent_at_time(-1), - retries(0) {} - -NackModule::NackModule(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), - running_(true), - initialized_(false), - rtt_ms_(kDefaultRttMs), - last_seq_num_(0), - next_process_time_ms_(-1) { - RTC_DCHECK(clock_); - RTC_DCHECK(nack_sender_); - RTC_DCHECK(keyframe_request_sender_); -} - -void NackModule::OnReceivedPacket(const VCMPacket& packet) { - rtc::CritScope lock(&crit_); - if (!running_) - return; - uint16_t seq_num = packet.seqNum; - // 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; - bool is_keyframe = packet.isFirstPacket && packet.frameType == kVideoFrameKey; - - if (!initialized_) { - last_seq_num_ = seq_num; - if (is_keyframe) - keyframe_list_.insert(seq_num); - initialized_ = true; - return; - } - - if (seq_num == last_seq_num_) - return; - - if (AheadOf(last_seq_num_, seq_num)) { - // An out of order packet has been received. - nack_list_.erase(seq_num); - if (!is_retransmitted) - UpdateReorderingStatistics(seq_num); - return; - } else { - AddPacketsToNack(last_seq_num_ + 1, seq_num); - last_seq_num_ = seq_num; - - // 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); - - // Are there any nacks that are waiting for this seq_num. - std::vector nack_batch = GetNackBatch(kSeqNumOnly); - if (!nack_batch.empty()) - nack_sender_->SendNack(nack_batch); - } -} - -void NackModule::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)); -} - -void NackModule::UpdateRtt(int64_t rtt_ms) { - rtc::CritScope lock(&crit_); - rtt_ms_ = rtt_ms; -} - -void NackModule::Stop() { - rtc::CritScope lock(&crit_); - running_ = false; -} - -int64_t NackModule::TimeUntilNextProcess() { - rtc::CritScope lock(&crit_); - return std::max(next_process_time_ms_ - clock_->TimeInMilliseconds(), - 0); -} - -void NackModule::Process() { - rtc::CritScope lock(&crit_); - if (!running_) - return; - - // 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; - } - - std::vector nack_batch = GetNackBatch(kTimeOnly); - if (!nack_batch.empty() && nack_sender_ != nullptr) - nack_sender_->SendNack(nack_batch); -} - -bool NackModule::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. - RTC_DCHECK(it != nack_list_.end()); - 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 NackModule::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(); - 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) { - NackInfo nack_info(seq_num, seq_num + WaitNumberOfPackets(0.5)); - RTC_DCHECK(nack_list_.find(seq_num) == nack_list_.end()); - nack_list_[seq_num] = nack_info; - } -} - -std::vector NackModule::GetNackBatch(NackFilterOptions options) { - bool consider_seq_num = options != kTimeOnly; - bool consider_timestamp = options != kSeqNumOnly; - int64_t now_ms = clock_->TimeInMilliseconds(); - std::vector nack_batch; - auto it = nack_list_.begin(); - while (it != nack_list_.end()) { - if (consider_seq_num && it->second.sent_at_time == -1 && - AheadOrAt(last_seq_num_, it->second.send_at_seq_num)) { - nack_batch.emplace_back(it->second.seq_num); - ++it->second.retries; - it->second.sent_at_time = now_ms; - if (it->second.retries >= kMaxNackRetries) { - 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; - } - - if (consider_timestamp && it->second.sent_at_time + rtt_ms_ <= now_ms) { - nack_batch.emplace_back(it->second.seq_num); - ++it->second.retries; - it->second.sent_at_time = now_ms; - if (it->second.retries >= kMaxNackRetries) { - 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 NackModule::UpdateReorderingStatistics(uint16_t seq_num) { - RTC_DCHECK(AheadOf(last_seq_num_, seq_num)); - uint16_t diff = ReverseDiff(last_seq_num_, seq_num); - reordering_histogram_.Add(diff); -} - -int NackModule::WaitNumberOfPackets(float probability) const { - if (reordering_histogram_.NumValues() == 0) - return 0; - return reordering_histogram_.InverseCdf(probability); -} - -} // namespace webrtc diff --git a/webrtc/modules/video_coding/nack_module.h b/webrtc/modules/video_coding/nack_module.h deleted file mode 100644 index 32692bef69..0000000000 --- a/webrtc/modules/video_coding/nack_module.h +++ /dev/null @@ -1,101 +0,0 @@ -/* - * 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 WEBRTC_MODULES_VIDEO_CODING_NACK_MODULE_H_ -#define WEBRTC_MODULES_VIDEO_CODING_NACK_MODULE_H_ - -#include -#include -#include - -#include "webrtc/base/criticalsection.h" -#include "webrtc/base/mod_ops.h" -#include "webrtc/base/thread_annotations.h" -#include "webrtc/modules/include/module.h" -#include "webrtc/modules/video_coding/include/video_coding_defines.h" -#include "webrtc/modules/video_coding/packet.h" -#include "webrtc/modules/video_coding/histogram.h" -#include "webrtc/system_wrappers/include/clock.h" - -namespace webrtc { - -class NackModule : public Module { - public: - NackModule(Clock* clock, - NackSender* nack_sender, - KeyFrameRequestSender* keyframe_request_sender); - - void OnReceivedPacket(const VCMPacket& packet); - void ClearUpTo(uint16_t seq_num); - void UpdateRtt(int64_t rtt_ms); - void Stop(); - - // 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); - - uint16_t seq_num; - uint16_t send_at_seq_num; - int64_t sent_at_time; - int retries; - }; - - struct SeqNumComparator { - bool operator()(uint16_t s1, uint16_t s2) const { return AheadOf(s2, s1); } - }; - - void AddPacketsToNack(uint16_t seq_num_start, uint16_t seq_num_end) - EXCLUSIVE_LOCKS_REQUIRED(crit_); - - // Removes packets from the nack list until the next keyframe. Returns true - // if packets were removed. - bool RemovePacketsUntilKeyFrame() EXCLUSIVE_LOCKS_REQUIRED(crit_); - std::vector GetNackBatch(NackFilterOptions options) - EXCLUSIVE_LOCKS_REQUIRED(crit_); - - // Update the reordering distribution. - void UpdateReorderingStatistics(uint16_t seq_num) - 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 - EXCLUSIVE_LOCKS_REQUIRED(crit_); - - rtc::CriticalSection crit_; - Clock* const clock_; - NackSender* const nack_sender_; - KeyFrameRequestSender* const keyframe_request_sender_; - - std::map nack_list_ GUARDED_BY(crit_); - std::set keyframe_list_ GUARDED_BY(crit_); - video_coding::Histogram reordering_histogram_ GUARDED_BY(crit_); - bool running_ GUARDED_BY(crit_); - bool initialized_ GUARDED_BY(crit_); - int64_t rtt_ms_ GUARDED_BY(crit_); - uint16_t last_seq_num_ GUARDED_BY(crit_); - int64_t next_process_time_ms_ GUARDED_BY(crit_); -}; - -} // namespace webrtc - -#endif // WEBRTC_MODULES_VIDEO_CODING_NACK_MODULE_H_ diff --git a/webrtc/modules/video_coding/nack_module_unittest.cc b/webrtc/modules/video_coding/nack_module_unittest.cc deleted file mode 100644 index 189a1beba0..0000000000 --- a/webrtc/modules/video_coding/nack_module_unittest.cc +++ /dev/null @@ -1,292 +0,0 @@ -/* - * 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 -#include "testing/gtest/include/gtest/gtest.h" -#include "webrtc/modules/video_coding/include/video_coding_defines.h" -#include "webrtc/modules/video_coding/nack_module.h" -#include "webrtc/system_wrappers/include/clock.h" -#include "webrtc/base/mod_ops.h" - -namespace webrtc { -class TestNackModule : public ::testing::Test, - public NackSender, - public KeyFrameRequestSender { - protected: - TestNackModule() - : clock_(new SimulatedClock(0)), - nack_module_(clock_.get(), this, this), - keyframes_requested_(0) {} - - void SendNack(const std::vector& sequence_numbers) override { - sent_nacks_.insert(sent_nacks_.end(), sequence_numbers.begin(), - sequence_numbers.end()); - } - - void RequestKeyFrame() override { ++keyframes_requested_; } - - std::unique_ptr clock_; - NackModule nack_module_; - std::vector sent_nacks_; - int keyframes_requested_; -}; - -TEST_F(TestNackModule, NackOnePacket) { - VCMPacket packet; - packet.seqNum = 1; - nack_module_.OnReceivedPacket(packet); - packet.seqNum = 3; - nack_module_.OnReceivedPacket(packet); - EXPECT_EQ(1u, sent_nacks_.size()); - EXPECT_EQ(2, sent_nacks_[0]); -} - -TEST_F(TestNackModule, WrappingSeqNum) { - VCMPacket packet; - packet.seqNum = 0xfffe; - nack_module_.OnReceivedPacket(packet); - packet.seqNum = 1; - nack_module_.OnReceivedPacket(packet); - EXPECT_EQ(2u, sent_nacks_.size()); - EXPECT_EQ(0xffff, sent_nacks_[0]); - EXPECT_EQ(0, sent_nacks_[1]); -} - -TEST_F(TestNackModule, WrappingSeqNumClearToKeyframe) { - VCMPacket packet; - packet.seqNum = 0xfffe; - nack_module_.OnReceivedPacket(packet); - packet.seqNum = 1; - nack_module_.OnReceivedPacket(packet); - EXPECT_EQ(2u, sent_nacks_.size()); - EXPECT_EQ(0xffff, sent_nacks_[0]); - EXPECT_EQ(0, sent_nacks_[1]); - - sent_nacks_.clear(); - packet.frameType = kVideoFrameKey; - packet.isFirstPacket = true; - packet.seqNum = 2; - nack_module_.OnReceivedPacket(packet); - EXPECT_EQ(0u, sent_nacks_.size()); - - packet.seqNum = 501; - nack_module_.OnReceivedPacket(packet); - 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(); - packet.frameType = kVideoFrameDelta; - packet.seqNum = 1001; - nack_module_.OnReceivedPacket(packet); - 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(); - packet.seqNum = 1004; - nack_module_.OnReceivedPacket(packet); - 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. - packet.seqNum = 1007; - nack_module_.OnReceivedPacket(packet); - 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_F(TestNackModule, 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_F(TestNackModule, ResendNack) { - VCMPacket packet; - packet.seqNum = 1; - nack_module_.OnReceivedPacket(packet); - packet.seqNum = 3; - nack_module_.OnReceivedPacket(packet); - EXPECT_EQ(1u, sent_nacks_.size()); - EXPECT_EQ(2, sent_nacks_[0]); - - // Default RTT is 100 - clock_->AdvanceTimeMilliseconds(99); - nack_module_.Process(); - EXPECT_EQ(1u, sent_nacks_.size()); - - clock_->AdvanceTimeMilliseconds(1); - nack_module_.Process(); - EXPECT_EQ(2u, sent_nacks_.size()); - - nack_module_.UpdateRtt(50); - clock_->AdvanceTimeMilliseconds(100); - nack_module_.Process(); - EXPECT_EQ(3u, sent_nacks_.size()); - - clock_->AdvanceTimeMilliseconds(50); - nack_module_.Process(); - EXPECT_EQ(4u, sent_nacks_.size()); - - packet.seqNum = 2; - nack_module_.OnReceivedPacket(packet); - clock_->AdvanceTimeMilliseconds(50); - nack_module_.Process(); - EXPECT_EQ(4u, sent_nacks_.size()); -} - -TEST_F(TestNackModule, ResendPacketMaxRetries) { - VCMPacket packet; - packet.seqNum = 1; - nack_module_.OnReceivedPacket(packet); - packet.seqNum = 3; - nack_module_.OnReceivedPacket(packet); - EXPECT_EQ(1u, sent_nacks_.size()); - EXPECT_EQ(2, sent_nacks_[0]); - - for (size_t retries = 1; retries < 10; ++retries) { - clock_->AdvanceTimeMilliseconds(100); - nack_module_.Process(); - EXPECT_EQ(retries + 1, sent_nacks_.size()); - } - - clock_->AdvanceTimeMilliseconds(100); - nack_module_.Process(); - EXPECT_EQ(10u, sent_nacks_.size()); -} - -TEST_F(TestNackModule, TooLargeNackList) { - VCMPacket packet; - packet.seqNum = 0; - nack_module_.OnReceivedPacket(packet); - packet.seqNum = 1001; - nack_module_.OnReceivedPacket(packet); - EXPECT_EQ(1000u, sent_nacks_.size()); - EXPECT_EQ(0, keyframes_requested_); - packet.seqNum = 1003; - nack_module_.OnReceivedPacket(packet); - EXPECT_EQ(1000u, sent_nacks_.size()); - EXPECT_EQ(1, keyframes_requested_); - packet.seqNum = 1004; - nack_module_.OnReceivedPacket(packet); - EXPECT_EQ(1000u, sent_nacks_.size()); - EXPECT_EQ(1, keyframes_requested_); -} - -TEST_F(TestNackModule, TooLargeNackListWithKeyFrame) { - VCMPacket packet; - packet.seqNum = 0; - nack_module_.OnReceivedPacket(packet); - packet.seqNum = 1; - packet.isFirstPacket = true; - packet.frameType = kVideoFrameKey; - nack_module_.OnReceivedPacket(packet); - packet.seqNum = 1001; - packet.isFirstPacket = false; - packet.frameType = kVideoFrameKey; - nack_module_.OnReceivedPacket(packet); - EXPECT_EQ(999u, sent_nacks_.size()); - EXPECT_EQ(0, keyframes_requested_); - packet.seqNum = 1003; - nack_module_.OnReceivedPacket(packet); - EXPECT_EQ(1000u, sent_nacks_.size()); - EXPECT_EQ(0, keyframes_requested_); - packet.seqNum = 1005; - nack_module_.OnReceivedPacket(packet); - EXPECT_EQ(1000u, sent_nacks_.size()); - EXPECT_EQ(1, keyframes_requested_); -} - -TEST_F(TestNackModule, ClearUpTo) { - VCMPacket packet; - packet.seqNum = 0; - nack_module_.OnReceivedPacket(packet); - packet.seqNum = 100; - nack_module_.OnReceivedPacket(packet); - 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_F(TestNackModule, ClearUpToWrap) { - VCMPacket packet; - packet.seqNum = 0xfff0; - nack_module_.OnReceivedPacket(packet); - packet.seqNum = 0xf; - nack_module_.OnReceivedPacket(packet); - 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]); -} - -} // namespace webrtc diff --git a/webrtc/modules/video_coding/video_coding.gypi b/webrtc/modules/video_coding/video_coding.gypi index 02c9c6b32b..94028a9a8b 100644 --- a/webrtc/modules/video_coding/video_coding.gypi +++ b/webrtc/modules/video_coding/video_coding.gypi @@ -36,7 +36,6 @@ 'frame_buffer.h', 'generic_decoder.h', 'generic_encoder.h', - 'histogram.h', 'inter_frame_delay.h', 'internal_defines.h', 'jitter_buffer.h', @@ -45,7 +44,6 @@ 'media_opt_util.h', 'media_optimization.h', 'nack_fec_tables.h', - 'nack_module.h', 'packet.h', 'qm_select_data.h', 'qm_select.h', @@ -67,12 +65,10 @@ 'generic_decoder.cc', 'generic_encoder.cc', 'inter_frame_delay.cc', - 'histogram.cc', 'jitter_buffer.cc', 'jitter_estimator.cc', 'media_opt_util.cc', 'media_optimization.cc', - 'nack_module.cc', 'packet.cc', 'qm_select.cc', 'receiver.cc',