Delete VieRemb class, move functionality to PacketRouter.

webrtc/modules/pacing/packet_router.cc

Index: webrtc/modules/pacing/packet_router.cc
diff --git a/webrtc/modules/pacing/packet_router.cc b/webrtc/modules/pacing/packet_router.cc
index ab86bfa4bd82551f8014cd5652540e743577ce8d..c0320875fefac8f932b90f1cb74b72075a1e7fd2 100644
--- a/webrtc/modules/pacing/packet_router.cc
+++ b/webrtc/modules/pacing/packet_router.cc
@@ -12,6 +12,7 @@
 
 #include "webrtc/base/atomicops.h"
 #include "webrtc/base/checks.h"
+#include "webrtc/base/timeutils.h"
 #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h"
 #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
@@ -121,7 +122,52 @@ uint16_t PacketRouter::AllocateSequenceNumber() {
   return new_seq;
 }
 
-bool PacketRouter::SendFeedback(rtcp::TransportFeedback* packet) {
+void PacketRouter::OnReceiveBitrateChanged(const std::vector<uint32_t>& ssrcs,
+                                           uint32_t bitrate_bps) {
+  const int kRembSendIntervalMs = 200;
+
+  // % threshold for if we should send a new REMB asap.
+  const uint32_t kSendThresholdPercent = 97;
+
+  int64_t now_ms = rtc::TimeMillis();
+  {
+    rtc::CritScope lock(&remb_crit_);
+
+    // If we already have an estimate, check if the new total estimate is below
+    // kSendThresholdPercent of the previous estimate.
+    if (last_send_bitrate_bps_ > 0) {
+      uint32_t new_remb_bitrate_bps =
+          last_send_bitrate_bps_ - bitrate_bps_ + bitrate_bps;
+
+      if (new_remb_bitrate_bps <
+          kSendThresholdPercent * last_send_bitrate_bps_ / 100) {
+        // The new bitrate estimate is less than kSendThresholdPercent % of the
+        // last report. Send a REMB asap.
+        last_remb_time_ms_ = now_ms - kRembSendIntervalMs;
+      }
+    }
+    bitrate_bps_ = bitrate_bps;
+
+    if (now_ms - last_remb_time_ms_ < kRembSendIntervalMs) {
+      return;
+    }
+    // NOTE: Updated if we intend to send the data; we might not have
+    // a module to actually send it.
+    last_remb_time_ms_ = now_ms;
+    last_send_bitrate_bps_ = bitrate_bps;
+  }
+  {

[danilchap, 2017/04/03 16:31:34]

Do you mind splitting this block into own public function?
bool PacketRouter::SendRemb(bitrate_bps, ssrcs) {
  rtc::CritScope cs(&modules_crit_);
  RtpRtcp* remb_module = nullptr;
  if (!rtp_send_modules_.empty())
    remb_module = rtp_send_modules_.front();
  else if (!rtp_receive_modules_.empty())
    remb_module = rtp_receive_modules_.front();
  else
    return false;
  remb_module->SetREMBData(bitrate_bps, ssrcs);
  // and may be to actually send remb...
  return remb_module->SendRtcp(kRtcpRemb) == 0;
}

[nisse-webrtc, 2017/04/04 09:46:44]

Not at all. (If possible, I'd actually prefer to not implement the
RtpReceiveBitrateUpdate call and move its logic elsewhere. But if you don't see
any easy way to do that, it has to wait for some other cl).
Is the last call needed? RTCPSender::SetREMBData ends with

  // Send a REMB immediately if we have a new REMB. The frequency of REMBs is
  // throttled by the caller.
  next_time_to_send_rtcp_ = clock_->TimeInMilliseconds();

[danilchap, 2017/04/04 10:18:27]

not strictly needed, but this is one of the way to resolve your TODO: Check REMB
status.

without explicit SendRtcp(Remb) it will be eventually send [on process thread]
only if remb_module was configured with remb (SetRembStatus(true)).
Calling SendRtcp(Remb) explicetly will ensure remb is actually sent.

On the other side there is risk here: SendRtcp is a large function that involve
taking bunch of locks, so need to be careful not add a deadlock.
Also, calling SendRtcp makes it harder to reason what threads rtcp messages are
send from.

not calling SendRtcp(Remb) probably a better decision in this CL.

+    rtc::CritScope lock(&modules_crit_);
+    // TODO(nisse): Check REMB status of the modules? Or add a loop,
+    // similar to SendTransportFeedback below?
+    if (!rtp_send_modules_.empty())
+      rtp_send_modules_.front()->SetREMBData(bitrate_bps, ssrcs);
+    else if (!rtp_receive_modules_.empty())
+      rtp_receive_modules_.front()->SetREMBData(bitrate_bps, ssrcs);
+  }
+}
+
+bool PacketRouter::SendTransportFeedback(rtcp::TransportFeedback* packet) {
   RTC_DCHECK(pacer_thread_checker_.CalledOnValidThread());
   rtc::CritScope cs(&modules_crit_);
   // Prefer send modules.