From 2f2971c6f3a2236e1d6be2cf9022459b0780bbd8 Mon Sep 17 00:00:00 2001 From: "holmer@google.com" Date: Mon, 20 Jun 2011 14:07:42 +0000 Subject: [PATCH] Fixed a bug in the BitRateStats class and at the same time rewrote it a bit. Review URL: http://webrtc-codereview.appspot.com/41001 git-svn-id: http://webrtc.googlecode.com/svn/trunk@103 4adac7df-926f-26a2-2b94-8c16560cd09d --- modules/rtp_rtcp/source/Bitrate.h | 20 +++-- modules/rtp_rtcp/source/bitrate.cc | 78 +++++++++---------- modules/rtp_rtcp/source/rtp_receiver_video.cc | 5 +- modules/rtp_rtcp/test/test_bwe/test_bwe.gyp | 38 +++++++++ modules/rtp_rtcp/test/test_bwe/unit_test.cc | 59 ++++++++++++++ 5 files changed, 150 insertions(+), 50 deletions(-) create mode 100644 modules/rtp_rtcp/test/test_bwe/test_bwe.gyp create mode 100644 modules/rtp_rtcp/test/test_bwe/unit_test.cc diff --git a/modules/rtp_rtcp/source/Bitrate.h b/modules/rtp_rtcp/source/Bitrate.h index 8859fcd6e7..61345634d6 100644 --- a/modules/rtp_rtcp/source/Bitrate.h +++ b/modules/rtp_rtcp/source/Bitrate.h @@ -14,7 +14,8 @@ #include "typedefs.h" #include "rtp_rtcp_config.h" // misc. defines (e.g. MAX_PACKET_LENGTH) #include "common_types.h" // Transport -#include "list_wrapper.h" +#include +#include namespace webrtc { class Bitrate @@ -54,10 +55,11 @@ private: struct DataTimeSizeTuple { - DataTimeSizeTuple(WebRtc_Word64 sizeBytes, WebRtc_Word64 timeCompleteMs) : - _sizeBytes(sizeBytes), _timeCompleteMs(timeCompleteMs) {} + DataTimeSizeTuple(WebRtc_UWord32 sizeBytes, WebRtc_Word64 timeCompleteMs) : + _sizeBytes(sizeBytes), + _timeCompleteMs(timeCompleteMs) {} - WebRtc_Word64 _sizeBytes; + WebRtc_UWord32 _sizeBytes; WebRtc_Word64 _timeCompleteMs; }; @@ -68,12 +70,14 @@ public: ~BitRateStats(); void Init(); - void Update(WebRtc_Word64 packetSizeBytes, WebRtc_Word64 nowMs); - WebRtc_UWord32 BitRateNow(); + void Update(WebRtc_UWord32 packetSizeBytes, WebRtc_Word64 nowMs); + WebRtc_UWord32 BitRate(WebRtc_Word64 nowMs); private: - ListWrapper _dataSamples; - WebRtc_UWord32 _avgSentBitRateBps; + void EraseOld(WebRtc_Word64 nowMs); + + std::list _dataSamples; + WebRtc_UWord32 _accumulatedBytes; }; } // namespace webrtc diff --git a/modules/rtp_rtcp/source/bitrate.cc b/modules/rtp_rtcp/source/bitrate.cc index 03e5ef7965..ebeb89a447 100644 --- a/modules/rtp_rtcp/source/bitrate.cc +++ b/modules/rtp_rtcp/source/bitrate.cc @@ -127,81 +127,77 @@ Bitrate::Process() } } - BitRateStats::BitRateStats() - :_dataSamples(), _avgSentBitRateBps(0) + :_dataSamples(), _accumulatedBytes(0) { } BitRateStats::~BitRateStats() { - ListItem* item = _dataSamples.First(); - while (item != NULL) + while (_dataSamples.size() > 0) { - delete static_cast(item->GetItem()); - _dataSamples.Erase(item); - item = _dataSamples.First(); + delete _dataSamples.front(); + _dataSamples.pop_front(); } } void BitRateStats::Init() { - _avgSentBitRateBps = 0; - - ListItem* item = _dataSamples.First(); - while (item != NULL) + _accumulatedBytes = 0; + while (_dataSamples.size() > 0) { - delete static_cast(item->GetItem()); - _dataSamples.Erase(item); - item = _dataSamples.First(); + delete _dataSamples.front(); + _dataSamples.pop_front(); } } -void BitRateStats::Update(WebRtc_Word64 packetSizeBytes, WebRtc_Word64 nowMs) +void BitRateStats::Update(WebRtc_UWord32 packetSizeBytes, WebRtc_Word64 nowMs) { - WebRtc_UWord32 sumBytes = 0; - WebRtc_Word64 timeOldest = nowMs; // Find an empty slot for storing the new sample and at the same time // accumulate the history. - _dataSamples.PushFront(new DataTimeSizeTuple(packetSizeBytes, nowMs)); - ListItem* item = _dataSamples.First(); - while (item != NULL) + _dataSamples.push_back(new DataTimeSizeTuple(packetSizeBytes, nowMs)); + _accumulatedBytes += packetSizeBytes; + EraseOld(nowMs); +} + +void BitRateStats::EraseOld(WebRtc_Word64 nowMs) +{ + while (_dataSamples.size() > 0) { - const DataTimeSizeTuple* sample = static_cast(item->GetItem()); - if (nowMs - sample->_timeCompleteMs < BITRATE_AVERAGE_WINDOW) + if (nowMs - _dataSamples.front()->_timeCompleteMs > + BITRATE_AVERAGE_WINDOW) { - sumBytes += static_cast(sample->_sizeBytes); - item = _dataSamples.Next(item); + // Delete old sample + _accumulatedBytes -= _dataSamples.front()->_sizeBytes; + delete _dataSamples.front(); + _dataSamples.pop_front(); } else { - // Delete old sample - delete sample; - ListItem* itemToErase = item; - item = _dataSamples.Next(item); - _dataSamples.Erase(itemToErase); + break; } } - const ListItem* oldest = _dataSamples.Last(); - if (oldest != NULL) +} + +WebRtc_UWord32 BitRateStats::BitRate(WebRtc_Word64 nowMs) +{ + // Calculate the average bit rate the past BITRATE_AVERAGE_WINDOW ms. + // Removes any old samples from the list. + EraseOld(nowMs); + WebRtc_Word64 timeOldest = nowMs; + if (_dataSamples.size() > 0) { - timeOldest = - static_cast(oldest->GetItem())->_timeCompleteMs; + timeOldest = _dataSamples.front()->_timeCompleteMs; } // Update average bit rate float denom = static_cast(nowMs - timeOldest); - if (denom < 1.0) + if (nowMs == timeOldest) { // Calculate with a one second window when we haven't // received more than one packet. denom = 1000.0; } - _avgSentBitRateBps = static_cast(sumBytes * 8.0f * 1000.0f / denom + 0.5f); -} - -WebRtc_UWord32 BitRateStats::BitRateNow() -{ - Update(-1, TickTime::MillisecondTimestamp()); - return static_cast(_avgSentBitRateBps + 0.5f); + return static_cast(_accumulatedBytes * 8.0f * 1000.0f / + denom + 0.5f); } } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_receiver_video.cc b/modules/rtp_rtcp/source/rtp_receiver_video.cc index 76e5ce3148..ca036d8e77 100644 --- a/modules/rtp_rtcp/source/rtp_receiver_video.cc +++ b/modules/rtp_rtcp/source/rtp_receiver_video.cc @@ -298,7 +298,10 @@ RTPReceiverVideo::ParseVideoCodecSpecific(WebRtcRTPHeader* rtpHeader, // Update the remote rate control object and update the overuse // detector with the current rate control region. _criticalSectionReceiverVideo.Enter(); - const RateControlInput input(_overUseDetector.State(), _videoBitRate.BitRateNow(), _overUseDetector.NoiseVar()); + const RateControlInput input(_overUseDetector.State(), + _videoBitRate.BitRate( + TickTime::MillisecondTimestamp()), + _overUseDetector.NoiseVar()); _criticalSectionReceiverVideo.Leave(); // Call the callback outside critical section diff --git a/modules/rtp_rtcp/test/test_bwe/test_bwe.gyp b/modules/rtp_rtcp/test/test_bwe/test_bwe.gyp new file mode 100644 index 0000000000..cce7f42d7e --- /dev/null +++ b/modules/rtp_rtcp/test/test_bwe/test_bwe.gyp @@ -0,0 +1,38 @@ +# Copyright (c) 2011 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. + +{ + 'includes': [ + '../../../../common_settings.gypi', # Common settings + ], + 'targets': [ + { + 'target_name': 'test_bwe', + 'type': 'executable', + 'dependencies': [ + '../../source/rtp_rtcp.gyp:rtp_rtcp', + '../../../../system_wrappers/source/system_wrappers.gyp:system_wrappers', + '../../../../../testing/gtest.gyp:gtest', + '../../../../../testing/gtest.gyp:gtest_main', + ], + 'include_dirs': [ + '../../source', + ], + 'sources': [ + 'unit_test.cc', + '../../source/bitrate.cc', + ], + }, + ], +} + +# Local Variables: +# tab-width:2 +# indent-tabs-mode:nil +# End: +# vim: set expandtab tabstop=2 shiftwidth=2: diff --git a/modules/rtp_rtcp/test/test_bwe/unit_test.cc b/modules/rtp_rtcp/test/test_bwe/unit_test.cc new file mode 100644 index 0000000000..73f64f1e42 --- /dev/null +++ b/modules/rtp_rtcp/test/test_bwe/unit_test.cc @@ -0,0 +1,59 @@ +/* + * Copyright (c) 2011 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. + */ + + +/* + * This file includes unit tests for the bandwidth estimation and management + */ + +#include + +#include "typedefs.h" +#include "Bitrate.h" + +namespace { + +using webrtc::BitRateStats; + +class BitRateStatsTest : public ::testing::Test +{ +protected: + BitRateStatsTest() {}; + BitRateStats bitRate; +}; + +TEST_F(BitRateStatsTest, TestStrictMode) +{ + WebRtc_Word64 nowMs = 0; + // Should be initialized to 0. + EXPECT_EQ(0, bitRate.BitRate(nowMs)); + bitRate.Update(1500, nowMs); + // Expecting 12 kbps given a 1000 window with one 1500 bytes packet. + EXPECT_EQ(12000, bitRate.BitRate(nowMs)); + bitRate.Init(); + // Expecting 0 after init. + EXPECT_EQ(0, bitRate.BitRate(nowMs)); + for (int i = 0; i < 100000; ++i) + { + if (nowMs % 10 == 0) + bitRate.Update(1500, nowMs); + // Approximately 1200 kbps expected. Not exact since when packets + // are removed we will jump 10 ms to the next packet. + if (nowMs > 0 && nowMs % 2000 == 0) + EXPECT_NEAR(1200000, bitRate.BitRate(nowMs), 6000); + nowMs += 1; + } + nowMs += 2000; + // The window is 2 seconds. If nothing has been received for that time + // the estimate should be 0. + EXPECT_EQ(0, bitRate.BitRate(nowMs)); +} + +}