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

@@ -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 <stdio.h>
+#include <list>
 
 namespace webrtc {
 class Bitrate
@@ -54,10 +55,11 @@ private:
 
 struct DataTimeSizeTuple
 {
-    DataTimeSizeTuple(WebRtc_Word64 sizeBytes, WebRtc_Word64 timeCompleteMs) :
+    DataTimeSizeTuple(WebRtc_UWord32 sizeBytes, WebRtc_Word64 timeCompleteMs) :
-                            _sizeBytes(sizeBytes), _timeCompleteMs(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);
+    void Update(WebRtc_UWord32 packetSizeBytes, WebRtc_Word64 nowMs);
-    WebRtc_UWord32 BitRateNow();
+    WebRtc_UWord32 BitRate(WebRtc_Word64 nowMs);
 
 private:
-    ListWrapper              _dataSamples;
+    void EraseOld(WebRtc_Word64 nowMs);
-    WebRtc_UWord32        _avgSentBitRateBps;
+
+    std::list<DataTimeSizeTuple*> _dataSamples;
+    WebRtc_UWord32                _accumulatedBytes;
 };
 } // namespace webrtc
 

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 (_dataSamples.size() > 0)
-    while (item != NULL)
     {
-        delete static_cast<DataTimeSizeTuple*>(item->GetItem());
+        delete _dataSamples.front();
-        _dataSamples.Erase(item);
+        _dataSamples.pop_front();
-        item = _dataSamples.First();
     }
 }
 
 void BitRateStats::Init()
 {
-    _avgSentBitRateBps = 0;
+    _accumulatedBytes = 0;
-
+    while (_dataSamples.size() > 0)
-    ListItem* item = _dataSamples.First();
-    while (item != NULL)
     {
-        delete static_cast<DataTimeSizeTuple*>(item->GetItem());
+        delete _dataSamples.front();
-        _dataSamples.Erase(item);
+        _dataSamples.pop_front();
-        item = _dataSamples.First();
     }
 }
 
-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));
+    _dataSamples.push_back(new DataTimeSizeTuple(packetSizeBytes, nowMs));
-    ListItem* item = _dataSamples.First();
+    _accumulatedBytes += packetSizeBytes;
-    while (item != NULL)
+    EraseOld(nowMs);
+}
+
+void BitRateStats::EraseOld(WebRtc_Word64 nowMs)
+{
+    while (_dataSamples.size() > 0)
     {
-        const DataTimeSizeTuple* sample = static_cast<DataTimeSizeTuple*>(item->GetItem());
+        if (nowMs - _dataSamples.front()->_timeCompleteMs >
-        if (nowMs - sample->_timeCompleteMs < BITRATE_AVERAGE_WINDOW)
+                                                        BITRATE_AVERAGE_WINDOW)
         {
-            sumBytes += static_cast<WebRtc_UWord32>(sample->_sizeBytes);
+            // Delete old sample
-            item = _dataSamples.Next(item);
+            _accumulatedBytes -= _dataSamples.front()->_sizeBytes;
+            delete _dataSamples.front();
+            _dataSamples.pop_front();
         }
         else
         {
-            // Delete old sample
+            break;
-            delete sample;
-            ListItem* itemToErase = item;
-            item = _dataSamples.Next(item);
-            _dataSamples.Erase(itemToErase);
         }
     }
-    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 =
+        timeOldest = _dataSamples.front()->_timeCompleteMs;
-           static_cast<DataTimeSizeTuple*>(oldest->GetItem())->_timeCompleteMs;
     }
     // Update average bit rate
     float denom = static_cast<float>(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<WebRtc_UWord32>(sumBytes * 8.0f * 1000.0f / denom + 0.5f);
+    return static_cast<WebRtc_UWord32>(_accumulatedBytes * 8.0f * 1000.0f /
-}
+                                       denom + 0.5f);
-
-WebRtc_UWord32 BitRateStats::BitRateNow()
-{
-    Update(-1, TickTime::MillisecondTimestamp());
-    return static_cast<WebRtc_UWord32>(_avgSentBitRateBps + 0.5f);
 }
 } // namespace webrtc

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

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:

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 <gtest/gtest.h>
+
+#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));
+}
+
+}