Move SetTargetSendBitrates logic from default module to payload router.

This cl just moves the logic form the default module
SetTargetSendBitrates to PayloadRouter. There might be glitch / mismatch
in size between trate the vector and rtp modules. This was the same in
the default module and is quite hard to protect from before we have the
new video API.

I also removed some test form rtp_rtcp_impl_unittest that were affected
by this change. The test tests code that isn't implemented, hence the
DISABLED_, and this will never be implemented in the RTP module, rather
the payload router in the future.

BUG=769
R=pbos@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/42419004

Cr-Commit-Position: refs/heads/master@{#8453}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8453 4adac7df-926f-26a2-2b94-8c16560cd09d

webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h

@@ -607,8 +607,7 @@ class RtpRtcp : public Module {
     /*
     *   Set the target send bitrate
     */
-    virtual void SetTargetSendBitrate(
+    virtual void SetTargetSendBitrate(uint32_t bitrate_bps) = 0;
-        const std::vector<uint32_t>& stream_bitrates) = 0;
 
     /*
     *   Turn on/off generic FEC

webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h

@@ -232,7 +232,7 @@ class MockRtpRtcp : public RtpRtcp {
   MOCK_METHOD1(SetAudioLevel,
       int32_t(const uint8_t level_dBov));
   MOCK_METHOD1(SetTargetSendBitrate,
-      void(const std::vector<uint32_t>& stream_bitrates));
+      void(uint32_t bitrate_bps));
   MOCK_METHOD3(SetGenericFECStatus,
       int32_t(const bool enable, const uint8_t payloadTypeRED, const uint8_t payloadTypeFEC));
   MOCK_METHOD3(GenericFECStatus,

webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc

@@ -894,34 +894,9 @@ int32_t ModuleRtpRtcpImpl::SendREDPayloadType(
   return rtp_sender_.RED(&payload_type);
 }
 
-void ModuleRtpRtcpImpl::SetTargetSendBitrate(
+void ModuleRtpRtcpImpl::SetTargetSendBitrate(uint32_t bitrate_bps) {
-    const std::vector<uint32_t>& stream_bitrates) {
+  DCHECK(!IsDefaultModule());
-  if (IsDefaultModule()) {
+  rtp_sender_.SetTargetBitrate(bitrate_bps);
-    CriticalSectionScoped lock(critical_section_module_ptrs_.get());
-    if (simulcast_) {
-      std::vector<ModuleRtpRtcpImpl*>::iterator it = child_modules_.begin();
-      for (size_t i = 0;
-           it != child_modules_.end() && i < stream_bitrates.size(); ++it) {
-        if ((*it)->SendingMedia()) {
-          RTPSender& rtp_sender = (*it)->rtp_sender_;
-          rtp_sender.SetTargetBitrate(stream_bitrates[i]);
-          ++i;
-        }
-      }
-    } else {
-      if (stream_bitrates.size() > 1)
-        return;
-      std::vector<ModuleRtpRtcpImpl*>::iterator it = child_modules_.begin();
-      for (; it != child_modules_.end(); ++it) {
-        RTPSender& rtp_sender = (*it)->rtp_sender_;
-        rtp_sender.SetTargetBitrate(stream_bitrates[0]);
-      }
-    }
-  } else {
-    if (stream_bitrates.size() > 1)
-      return;
-    rtp_sender_.SetTargetBitrate(stream_bitrates[0]);
-  }
 }
 
 int32_t ModuleRtpRtcpImpl::SetKeyFrameRequestMethod(

webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h

@@ -288,8 +288,7 @@ class ModuleRtpRtcpImpl : public RtpRtcp {
   // Send a request for a keyframe.
   virtual int32_t RequestKeyFrame() OVERRIDE;
 
-  virtual void SetTargetSendBitrate(
+  virtual void SetTargetSendBitrate(uint32_t bitrate_bps) OVERRIDE;
-      const std::vector<uint32_t>& stream_bitrates) OVERRIDE;
 
   virtual int32_t SetGenericFECStatus(bool enable,
                                       uint8_t payload_type_red,

webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc

@@ -30,7 +30,6 @@ namespace webrtc {
 namespace {
 const uint32_t kSenderSsrc = 0x12345;
 const uint32_t kReceiverSsrc = 0x23456;
-const uint32_t kSenderRtxSsrc = 0x32345;
 const int64_t kOneWayNetworkDelayMs = 100;
 const uint8_t kBaseLayerTid = 0;
 const uint8_t kHigherLayerTid = 1;
@@ -521,253 +520,4 @@ TEST_F(RtpRtcpImplTest, UniqueNackRequests) {
   EXPECT_EQ(75, sender_.RtcpReceived().UniqueNackRequestsInPercent());
 }
 
-class RtpSendingTestTransport : public Transport {
- public:
-  void ResetCounters() { bytes_received_.clear(); }
-
-  virtual int SendPacket(int channel,
-                         const void* data,
-                         size_t length) OVERRIDE {
-    RTPHeader header;
-    scoped_ptr<RtpHeaderParser> parser(RtpHeaderParser::Create());
-    EXPECT_TRUE(parser->Parse(static_cast<const uint8_t*>(data),
-                              static_cast<size_t>(length),
-                              &header));
-    bytes_received_[header.ssrc] += length;
-    ++packets_received_[header.ssrc];
-    return static_cast<int>(length);
-  }
-
-  virtual int SendRTCPPacket(int channel,
-                             const void* data,
-                             size_t length) OVERRIDE {
-    return static_cast<int>(length);
-  }
-
-  int GetPacketsReceived(uint32_t ssrc) const {
-    std::map<uint32_t, int>::const_iterator it = packets_received_.find(ssrc);
-    if (it == packets_received_.end())
-      return 0;
-    return it->second;
-  }
-
-  int GetBytesReceived(uint32_t ssrc) const {
-    std::map<uint32_t, int>::const_iterator it = bytes_received_.find(ssrc);
-    if (it == bytes_received_.end())
-      return 0;
-    return it->second;
-  }
-
-  int GetTotalBytesReceived() const {
-    int sum = 0;
-    for (std::map<uint32_t, int>::const_iterator it = bytes_received_.begin();
-         it != bytes_received_.end();
-         ++it) {
-      sum += it->second;
-    }
-    return sum;
-  }
-
- private:
-  std::map<uint32_t, int> bytes_received_;
-  std::map<uint32_t, int> packets_received_;
-};
-
-class RtpSendingTest : public ::testing::Test {
- protected:
-  // Map from SSRC to number of received packets and bytes.
-  typedef std::map<uint32_t, std::pair<int, int> > PaddingMap;
-
-  RtpSendingTest() {
-    // Send module.
-    RtpRtcp::Configuration config;
-    config.audio = false;
-    config.clock = Clock::GetRealTimeClock();
-    config.outgoing_transport = &transport_;
-    config.receive_statistics = receive_statistics_.get();
-    config.rtt_stats = &rtt_stats_;
-    config.paced_sender = &pacer_;
-    memset(&codec_, 0, sizeof(VideoCodec));
-    codec_.plType = 100;
-    strncpy(codec_.plName, "VP8", 3);
-    codec_.numberOfSimulcastStreams = 3;
-    codec_.simulcastStream[0].width = 320;
-    codec_.simulcastStream[0].height = 180;
-    codec_.simulcastStream[0].maxBitrate = 300;
-    codec_.simulcastStream[1].width = 640;
-    codec_.simulcastStream[1].height = 360;
-    codec_.simulcastStream[1].maxBitrate = 600;
-    codec_.simulcastStream[2].width = 1280;
-    codec_.simulcastStream[2].height = 720;
-    codec_.simulcastStream[2].maxBitrate = 1200;
-    // We need numberOfSimulcastStreams + 1 RTP modules since we need one
-    // default module.
-    for (int i = 0; i < codec_.numberOfSimulcastStreams + 1; ++i) {
-      RtpRtcp* sender = RtpRtcp::CreateRtpRtcp(config);
-      EXPECT_EQ(0, sender->RegisterSendPayload(codec_));
-      EXPECT_EQ(0, sender->SetSendingStatus(true));
-      sender->SetSendingMediaStatus(true);
-      sender->SetSSRC(kSenderSsrc + i);
-      sender->SetRemoteSSRC(kReceiverSsrc + i);
-      senders_.push_back(sender);
-      config.default_module = senders_[0];
-    }
-    std::vector<uint32_t> bitrates;
-    bitrates.push_back(codec_.simulcastStream[0].maxBitrate);
-    bitrates.push_back(codec_.simulcastStream[1].maxBitrate);
-    bitrates.push_back(codec_.simulcastStream[2].maxBitrate);
-    senders_[0]->SetTargetSendBitrate(bitrates);
-  }
-
-  ~RtpSendingTest() {
-    for (int i = senders_.size() - 1; i >= 0; --i) {
-      delete senders_[i];
-    }
-  }
-
-  void SendFrameOnSender(int sender_index,
-                         const uint8_t* payload,
-                         size_t length) {
-    RTPVideoHeader rtp_video_header = {
-        codec_.simulcastStream[sender_index].width,
-        codec_.simulcastStream[sender_index].height,
-        true,
-        0,
-        kRtpVideoVp8,
-        {}};
-    uint32_t seq_num = 0;
-    uint32_t ssrc = 0;
-    int64_t capture_time_ms = 0;
-    bool retransmission = false;
-    EXPECT_CALL(pacer_, SendPacket(_, _, _, _, _, _))
-        .WillRepeatedly(DoAll(SaveArg<1>(&ssrc),
-                              SaveArg<2>(&seq_num),
-                              SaveArg<3>(&capture_time_ms),
-                              SaveArg<5>(&retransmission),
-                              Return(true)));
-    EXPECT_EQ(0,
-              senders_[sender_index]->SendOutgoingData(kVideoFrameKey,
-                                                       codec_.plType,
-                                                       0,
-                                                       0,
-                                                       payload,
-                                                       length,
-                                                       NULL,
-                                                       &rtp_video_header));
-    EXPECT_TRUE(senders_[sender_index]->TimeToSendPacket(
-        ssrc, seq_num, capture_time_ms, retransmission));
-  }
-
-  void ExpectPadding(const PaddingMap& expected_padding) {
-    int expected_total_bytes = 0;
-    for (PaddingMap::const_iterator it = expected_padding.begin();
-         it != expected_padding.end();
-         ++it) {
-      int packets_received = transport_.GetBytesReceived(it->first);
-      if (it->second.first > 0) {
-        EXPECT_GE(packets_received, it->second.first)
-            << "On SSRC: " << it->first;
-      }
-      int bytes_received = transport_.GetBytesReceived(it->first);
-      expected_total_bytes += bytes_received;
-      if (it->second.second > 0) {
-        EXPECT_GE(bytes_received, it->second.second)
-            << "On SSRC: " << it->first;
-      } else {
-        EXPECT_EQ(0, bytes_received) << "On SSRC: " << it->first;
-      }
-    }
-    EXPECT_EQ(expected_total_bytes, transport_.GetTotalBytesReceived());
-  }
-
-  scoped_ptr<ReceiveStatistics> receive_statistics_;
-  RtcpRttStatsTestImpl rtt_stats_;
-  std::vector<RtpRtcp*> senders_;
-  RtpSendingTestTransport transport_;
-  NiceMock<MockPacedSender> pacer_;
-  VideoCodec codec_;
-};
-
-TEST_F(RtpSendingTest, DISABLED_RoundRobinPadding) {
-  // We have to send on an SSRC to be allowed to pad, since a marker bit must
-  // be sent prior to padding packets.
-  const uint8_t payload[200] = {0};
-  for (int i = 0; i < codec_.numberOfSimulcastStreams; ++i) {
-    SendFrameOnSender(i + 1, payload, sizeof(payload));
-  }
-  transport_.ResetCounters();
-  senders_[0]->TimeToSendPadding(500);
-  PaddingMap expected_padding;
-  expected_padding[kSenderSsrc + 1] = std::make_pair(2, 500);
-  expected_padding[kSenderSsrc + 2] = std::make_pair(0, 0);
-  expected_padding[kSenderSsrc + 3] = std::make_pair(0, 0);
-  ExpectPadding(expected_padding);
-  senders_[0]->TimeToSendPadding(1000);
-  expected_padding[kSenderSsrc + 2] = std::make_pair(4, 1000);
-  ExpectPadding(expected_padding);
-  senders_[0]->TimeToSendPadding(1500);
-  expected_padding[kSenderSsrc + 3] = std::make_pair(6, 1500);
-  ExpectPadding(expected_padding);
-}
-
-TEST_F(RtpSendingTest, DISABLED_RoundRobinPaddingRtx) {
-  // Enable RTX to allow padding to be sent prior to media.
-  for (int i = 1; i < codec_.numberOfSimulcastStreams + 1; ++i) {
-    // Abs-send-time is needed to be allowed to send padding prior to media,
-    // as otherwise the timestmap used for BWE will be broken.
-    senders_[i]->RegisterSendRtpHeaderExtension(kRtpExtensionAbsoluteSendTime,
-                                                1);
-    senders_[i]->SetRtxSendPayloadType(96);
-    senders_[i]->SetRtxSsrc(kSenderRtxSsrc + i);
-    senders_[i]->SetRtxSendStatus(kRtxRetransmitted);
-  }
-  transport_.ResetCounters();
-  senders_[0]->TimeToSendPadding(500);
-  PaddingMap expected_padding;
-  expected_padding[kSenderSsrc + 1] = std::make_pair(0, 0);
-  expected_padding[kSenderSsrc + 2] = std::make_pair(0, 0);
-  expected_padding[kSenderSsrc + 3] = std::make_pair(0, 0);
-  expected_padding[kSenderRtxSsrc + 1] = std::make_pair(2, 500);
-  expected_padding[kSenderRtxSsrc + 2] = std::make_pair(0, 0);
-  expected_padding[kSenderRtxSsrc + 3] = std::make_pair(0, 0);
-  ExpectPadding(expected_padding);
-  senders_[0]->TimeToSendPadding(1000);
-  expected_padding[kSenderRtxSsrc + 2] = std::make_pair(4, 500);
-  ExpectPadding(expected_padding);
-  senders_[0]->TimeToSendPadding(1500);
-
-  expected_padding[kSenderRtxSsrc + 3] = std::make_pair(6, 500);
-  ExpectPadding(expected_padding);
-}
-
-TEST_F(RtpSendingTest, DISABLED_RoundRobinPaddingRtxRedundantPayloads) {
-  for (int i = 1; i < codec_.numberOfSimulcastStreams + 1; ++i) {
-    senders_[i]->SetRtxSendPayloadType(96);
-    senders_[i]->SetRtxSsrc(kSenderRtxSsrc + i);
-    senders_[i]->SetRtxSendStatus(kRtxRetransmitted | kRtxRedundantPayloads);
-    senders_[i]->SetStorePacketsStatus(true, 100);
-  }
-  // First send payloads so that we have something to retransmit.
-  const size_t kPayloadSize = 500;
-  const uint8_t payload[kPayloadSize] = {0};
-  for (int i = 0; i < codec_.numberOfSimulcastStreams; ++i) {
-    SendFrameOnSender(i + 1, payload, sizeof(payload));
-  }
-  transport_.ResetCounters();
-  senders_[0]->TimeToSendPadding(500);
-  PaddingMap expected_padding;
-  expected_padding[kSenderSsrc + 1] = std::make_pair<int, int>(0, 0);
-  expected_padding[kSenderSsrc + 2] = std::make_pair<int, int>(0, 0);
-  expected_padding[kSenderSsrc + 3] = std::make_pair<int, int>(0, 0);
-  expected_padding[kSenderRtxSsrc + 1] = std::make_pair<int, int>(1, 500);
-  expected_padding[kSenderRtxSsrc + 2] = std::make_pair<int, int>(0, 0);
-  expected_padding[kSenderRtxSsrc + 3] = std::make_pair<int, int>(0, 0);
-  ExpectPadding(expected_padding);
-  senders_[0]->TimeToSendPadding(1000);
-  expected_padding[kSenderRtxSsrc + 2] = std::make_pair<int, int>(2, 1000);
-  ExpectPadding(expected_padding);
-  senders_[0]->TimeToSendPadding(1500);
-  expected_padding[kSenderRtxSsrc + 3] = std::make_pair<int, int>(3, 1500);
-  ExpectPadding(expected_padding);
-}
 }  // namespace webrtc

webrtc/video_engine/payload_router.cc

@@ -57,12 +57,15 @@ bool PayloadRouter::RoutePayload(FrameType frame_type,
                                  const RTPFragmentationHeader* fragmentation,
                                  const RTPVideoHeader* rtp_video_hdr) {
   CriticalSectionScoped cs(crit_.get());
-  DCHECK(rtp_video_hdr == NULL ||
-         rtp_video_hdr->simulcastIdx <= rtp_modules_.size());
-
   if (!active_ || rtp_modules_.empty())
     return false;
 
+  // The simulcast index might actually be larger than the number of modules in
+  // case the encoder was processing a frame during a codec reconfig.
+  if (rtp_video_hdr != NULL &&
+      rtp_video_hdr->simulcastIdx >= rtp_modules_.size())
+    return false;
+
   int stream_idx = 0;
   if (rtp_video_hdr != NULL)
     stream_idx = rtp_video_hdr->simulcastIdx;
@@ -85,6 +88,19 @@ bool PayloadRouter::TimeToSendPacket(uint32_t ssrc,
   return true;
 }
 
+void PayloadRouter::SetTargetSendBitrates(
+    const std::vector<uint32_t>& stream_bitrates) {
+  CriticalSectionScoped cs(crit_.get());
+  if (stream_bitrates.size() < rtp_modules_.size()) {
+    // There can be a size mis-match during codec reconfiguration.
+    return;
+  }
+  int idx = 0;
+  for (auto* rtp_module : rtp_modules_) {
+    rtp_module->SetTargetSendBitrate(stream_bitrates[idx++]);
+  }
+}
+
 size_t PayloadRouter::TimeToSendPadding(size_t bytes) {
   CriticalSectionScoped cs(crit_.get());
   for(auto* rtp_module : rtp_modules_) {

webrtc/video_engine/payload_router.h

@@ -65,6 +65,10 @@ class PayloadRouter {
   // sent.
   size_t TimeToSendPadding(size_t bytes);
 
+  // Configures current target bitrate per module. 'stream_bitrates' is assumed
+  // to be in the same order as 'SetSendingRtpModules'.
+  void SetTargetSendBitrates(const std::vector<uint32_t>& stream_bitrates);
+
   // Returns the maximum allowed data payload length, given the configured MTU
   // and RTP headers.
   size_t MaxPayloadLength() const;

webrtc/video_engine/payload_router_unittest.cc

@@ -116,6 +116,7 @@ TEST_F(PayloadRouterTest, SendSimulcast) {
   EXPECT_TRUE(payload_router_->RoutePayload(frame_type_2, payload_type_2, 0, 0,
                                             &payload_2, 1, NULL, &rtp_hdr_2));
 
+  // Inactive.
   payload_router_->set_active(false);
   EXPECT_CALL(rtp_1, SendOutgoingData(_, _, _, _, _, _, _, _))
       .Times(0);
@@ -125,6 +126,16 @@ TEST_F(PayloadRouterTest, SendSimulcast) {
                                              &payload_1, 1, NULL, &rtp_hdr_1));
   EXPECT_FALSE(payload_router_->RoutePayload(frame_type_2, payload_type_2, 0, 0,
                                              &payload_2, 1, NULL, &rtp_hdr_2));
+
+  // Invalid simulcast index.
+  payload_router_->set_active(true);
+  EXPECT_CALL(rtp_1, SendOutgoingData(_, _, _, _, _, _, _, _))
+      .Times(0);
+  EXPECT_CALL(rtp_2, SendOutgoingData(_, _, _, _, _, _, _, _))
+      .Times(0);
+  rtp_hdr_1.simulcastIdx = 2;
+  EXPECT_FALSE(payload_router_->RoutePayload(frame_type_1, payload_type_1, 0, 0,
+                                             &payload_1, 1, NULL, &rtp_hdr_1));
 }
 
 TEST_F(PayloadRouterTest, MaxPayloadLength) {
@@ -257,7 +268,6 @@ TEST_F(PayloadRouterTest, TimeToSendPadding) {
   modules.push_back(&rtp_2);
   payload_router_->SetSendingRtpModules(modules);
 
-
   // Default configuration, sending padding on the first sending module.
   const size_t requested_padding_bytes = 1000;
   const size_t sent_padding_bytes = 890;
@@ -302,4 +312,39 @@ TEST_F(PayloadRouterTest, TimeToSendPadding) {
   EXPECT_EQ(static_cast<size_t>(0),
             payload_router_->TimeToSendPadding(requested_padding_bytes));
 }
+
+TEST_F(PayloadRouterTest, SetTargetSendBitrates) {
+  MockRtpRtcp rtp_1;
+  MockRtpRtcp rtp_2;
+  std::list<RtpRtcp*> modules;
+  modules.push_back(&rtp_1);
+  modules.push_back(&rtp_2);
+  payload_router_->SetSendingRtpModules(modules);
+
+  const uint32_t bitrate_1 = 10000;
+  const uint32_t bitrate_2 = 76543;
+  std::vector<uint32_t> bitrates (2, bitrate_1);
+  bitrates[1] = bitrate_2;
+  EXPECT_CALL(rtp_1, SetTargetSendBitrate(bitrate_1))
+      .Times(1);
+  EXPECT_CALL(rtp_2, SetTargetSendBitrate(bitrate_2))
+      .Times(1);
+  payload_router_->SetTargetSendBitrates(bitrates);
+
+  bitrates.resize(1);
+  EXPECT_CALL(rtp_1, SetTargetSendBitrate(bitrate_1))
+      .Times(0);
+  EXPECT_CALL(rtp_2, SetTargetSendBitrate(bitrate_2))
+      .Times(0);
+  payload_router_->SetTargetSendBitrates(bitrates);
+
+  bitrates.resize(3);
+  bitrates[1] = bitrate_2;
+  bitrates[2] = bitrate_1 + bitrate_2;
+  EXPECT_CALL(rtp_1, SetTargetSendBitrate(bitrate_1))
+      .Times(1);
+  EXPECT_CALL(rtp_2, SetTargetSendBitrate(bitrate_2))
+      .Times(1);
+  payload_router_->SetTargetSendBitrates(bitrates);
+  }
 }  // namespace webrtc

webrtc/video_engine/vie_encoder.cc

@@ -380,7 +380,7 @@ int32_t ViEEncoder::SetEncoder(const webrtc::VideoCodec& video_codec) {
       video_codec.startBitrate * 1000,
       video_codec.simulcastStream,
       video_codec.numberOfSimulcastStreams);
-  default_rtp_rtcp_->SetTargetSendBitrate(stream_bitrates);
+  send_payload_router_->SetTargetSendBitrates(stream_bitrates);
 
   {
     CriticalSectionScoped cs(data_cs_.get());
@@ -872,6 +872,7 @@ void ViEEncoder::OnNetworkChanged(uint32_t bitrate_bps,
   LOG(LS_VERBOSE) << "OnNetworkChanged, bitrate" << bitrate_bps
                   << " packet loss " << fraction_lost
                   << " rtt " << round_trip_time_ms;
+  DCHECK(send_payload_router_ != NULL);
   vcm_.SetChannelParameters(bitrate_bps, fraction_lost, round_trip_time_ms);
   bool video_is_suspended = vcm_.VideoSuspended();
   int bitrate_kbps = bitrate_bps / 1000;
@@ -924,7 +925,7 @@ void ViEEncoder::OnNetworkChanged(uint32_t bitrate_bps,
         bitrate_kbps,
         PacedSender::kDefaultPaceMultiplier * bitrate_kbps,
         pad_up_to_bitrate_kbps);
-    default_rtp_rtcp_->SetTargetSendBitrate(stream_bitrates);
+    send_payload_router_->SetTargetSendBitrates(stream_bitrates);
     if (video_suspended_ == video_is_suspended)
       return;
     video_suspended_ = video_is_suspended;