Refactor NACK bitrate allocation

webrtc/modules/rtp_rtcp/source/rtp_sender.h

Index: webrtc/modules/rtp_rtcp/source/rtp_sender.h
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.h b/webrtc/modules/rtp_rtcp/source/rtp_sender.h
index ffbcb817e709cb569da1ec09c0e5d43adfd14735..6c9c41aa645e00491d7272bc24cce9b8338d3dae 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender.h
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.h
@@ -20,10 +20,10 @@
 #include "webrtc/base/constructormagic.h"
 #include "webrtc/base/criticalsection.h"
 #include "webrtc/base/random.h"
+#include "webrtc/base/rate_statistics.h"
 #include "webrtc/base/thread_annotations.h"
 #include "webrtc/common_types.h"
 #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
-#include "webrtc/modules/rtp_rtcp/source/bitrate.h"
 #include "webrtc/modules/rtp_rtcp/source/playout_delay_oracle.h"
 #include "webrtc/modules/rtp_rtcp/source/rtp_header_extension.h"
 #include "webrtc/modules/rtp_rtcp/source/rtp_packet_history.h"
@@ -64,7 +64,7 @@ class RTPSenderInterface {
   virtual uint16_t SequenceNumber() const = 0;
   virtual size_t MaxPayloadLength() const = 0;
   virtual size_t MaxDataPayloadLength() const = 0;
-  virtual uint16_t ActualSendBitrateKbit() const = 0;
+  virtual uint16_t ActualSendBitrateKbit() = 0;
 
   virtual int32_t SendToNetwork(uint8_t* data_buffer,
                                 size_t payload_length,
@@ -99,11 +99,11 @@ class RTPSender : public RTPSenderInterface {
 
   void ProcessBitrate();
 
-  uint16_t ActualSendBitrateKbit() const override;
+  uint16_t ActualSendBitrateKbit() override;
 
   uint32_t VideoBitrateSent() const;
   uint32_t FecOverheadRate() const;
-  uint32_t NackOverheadRate() const;
+  uint32_t NackOverheadRate();
 
   void SetTargetBitrate(uint32_t bitrate);
   uint32_t GetTargetBitrate();
@@ -227,8 +227,6 @@ class RTPSender : public RTPSenderInterface {
 
   int32_t ReSendPacket(uint16_t packet_id, int64_t min_resend_time = 0);
 
-  bool ProcessNACKBitRate(uint32_t now);
-
   // Feedback to decide when to stop sending playout delay.
   void OnReceivedRtcpReportBlocks(const ReportBlockList& report_blocks);
 
@@ -315,7 +313,7 @@ class RTPSender : public RTPSenderInterface {
   void RegisterRtpStatisticsCallback(StreamDataCountersCallback* callback);
   StreamDataCountersCallback* GetRtpStatisticsCallback() const;
 
-  uint32_t BitrateSent() const;
+  uint32_t BitrateSent();
 
   void SetRtpState(const RtpState& rtp_state);
   RtpState GetRtpState() const;
@@ -340,8 +338,6 @@ class RTPSender : public RTPSenderInterface {
                          uint16_t sequence_number,
                          const std::vector<uint32_t>& csrcs) const;
 
-  void UpdateNACKBitRate(uint32_t bytes, int64_t now);
-
   bool PrepareAndSendPacket(uint8_t* buffer,
                             size_t length,
                             int64_t capture_time_ms,
@@ -406,44 +402,14 @@ class RTPSender : public RTPSenderInterface {
                       bool is_retransmit);
   bool IsFecPacket(const uint8_t* buffer, const RTPHeader& header) const;
 
-  class BitrateAggregator {
-   public:
-    explicit BitrateAggregator(BitrateStatisticsObserver* bitrate_callback);
-
-    void OnStatsUpdated() const;
-
-    Bitrate::Observer* total_bitrate_observer();
-    Bitrate::Observer* retransmit_bitrate_observer();
-    void set_ssrc(uint32_t ssrc);
-
-   private:
-    // We assume that these observers are called on the same thread, which is
-    // true for RtpSender as they are called on the Process thread.
-    class BitrateObserver : public Bitrate::Observer {
-     public:
-      explicit BitrateObserver(const BitrateAggregator& aggregator);
-
-      // Implements Bitrate::Observer.
-      void BitrateUpdated(const BitrateStatistics& stats) override;
-      const BitrateStatistics& statistics() const;
-
-     private:
-      BitrateStatistics statistics_;
-      const BitrateAggregator& aggregator_;
-    };
-
-    BitrateStatisticsObserver* const callback_;
-    BitrateObserver total_bitrate_observer_;
-    BitrateObserver retransmit_bitrate_observer_;
-    uint32_t ssrc_;
-  };
-
   Clock* const clock_;
   const int64_t clock_delta_ms_;
   Random random_ GUARDED_BY(send_critsect_);
 
-  BitrateAggregator bitrates_;
-  Bitrate total_bitrate_sent_;
+  rtc::CriticalSection stats_critsect_;

[danilchap, 2016/06/23 12:46:12]

may be use already existent statistics_crit_  instead (and avoid locking two
critsect in UpdateRtpStats)

[sprang_webrtc, 2016/06/28 09:12:33]

Done.

+  BitrateStatisticsObserver* const bitrate_callback_;
+  RateStatistics total_bitrate_sent_ GUARDED_BY(stats_critsect_);
+  RateStatistics nack_bitrate_sent_ GUARDED_BY(stats_critsect_);
 
   const bool audio_configured_;
   const std::unique_ptr<RTPSenderAudio> audio_;
@@ -470,11 +436,6 @@ class RTPSender : public RTPSenderInterface {
   bool video_rotation_active_;
   uint16_t transport_sequence_number_;
 
-  // NACK
-  uint32_t nack_byte_count_times_[NACK_BYTECOUNT_SIZE];
-  size_t nack_byte_count_[NACK_BYTECOUNT_SIZE];
-  Bitrate nack_bitrate_;
-
   // Tracks the current request for playout delay limits from application
   // and decides whether the current RTP frame should include the playout
   // delay extension on header.