Reland of Enable audio streams to send padding. (patchset #4 id:60001 of https://codereview.webrtc.org/2652893004/ )

Original issue's description:
> Enable audio streams to send padding.
>
> Useful if bitrate probing is to be used with audio streams.
>
> BUG=webrtc:7043
>
> Review-Url: https://codereview.webrtc.org/2652893004
> Cr-Commit-Position: refs/heads/master@{#16404}
> Committed: e35f89a484

BUG=webrtc:7043

Review-Url: https://codereview.webrtc.org/2675703002
Cr-Commit-Position: refs/heads/master@{#16433}

webrtc/modules/pacing/packet_router.cc

@@ -72,7 +72,7 @@ size_t PacketRouter::TimeToSendPadding(size_t bytes_to_send,
   rtc::CritScope cs(&modules_crit_);
   // Rtp modules are ordered by which stream can most benefit from padding.
   for (RtpRtcp* module : rtp_modules_) {
-    if (module->SendingMedia()) {
+    if (module->SendingMedia() && module->HasBweExtensions()) {
       size_t bytes_sent = module->TimeToSendPadding(
           bytes_to_send - total_bytes_sent, probe_cluster_id);
       total_bytes_sent += bytes_sent;

webrtc/modules/pacing/packet_router_unittest.cc

@@ -122,10 +122,12 @@ TEST_F(PacketRouterTest, TimeToSendPadding) {
   const size_t requested_padding_bytes = 1000;
   const size_t sent_padding_bytes = 890;
   EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true));
+  EXPECT_CALL(rtp_2, HasBweExtensions()).Times(1).WillOnce(Return(true));
   EXPECT_CALL(rtp_2, TimeToSendPadding(requested_padding_bytes, 111))
       .Times(1)
       .WillOnce(Return(sent_padding_bytes));
   EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(true));
+  EXPECT_CALL(rtp_1, HasBweExtensions()).Times(1).WillOnce(Return(true));
   EXPECT_CALL(rtp_1, TimeToSendPadding(
                          requested_padding_bytes - sent_padding_bytes, 111))
       .Times(1)
@@ -138,6 +140,7 @@ TEST_F(PacketRouterTest, TimeToSendPadding) {
   EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(false));
   EXPECT_CALL(rtp_2, TimeToSendPadding(requested_padding_bytes, _)).Times(0);
   EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(true));
+  EXPECT_CALL(rtp_1, HasBweExtensions()).Times(1).WillOnce(Return(true));
   EXPECT_CALL(rtp_1, TimeToSendPadding(_, _))
       .Times(1)
       .WillOnce(Return(sent_padding_bytes));
@@ -153,11 +156,25 @@ TEST_F(PacketRouterTest, TimeToSendPadding) {
   EXPECT_EQ(0u, packet_router_->TimeToSendPadding(requested_padding_bytes,
                                                   PacketInfo::kNotAProbe));
 
+  // Only one module has BWE extensions.
+  EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(true));
+  EXPECT_CALL(rtp_1, HasBweExtensions()).Times(1).WillOnce(Return(false));
+  EXPECT_CALL(rtp_1, TimeToSendPadding(requested_padding_bytes, _)).Times(0);
+  EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true));
+  EXPECT_CALL(rtp_2, HasBweExtensions()).Times(1).WillOnce(Return(true));
+  EXPECT_CALL(rtp_2, TimeToSendPadding(requested_padding_bytes, _))
+      .Times(1)
+      .WillOnce(Return(sent_padding_bytes));
+  EXPECT_EQ(sent_padding_bytes,
+            packet_router_->TimeToSendPadding(requested_padding_bytes,
+                                              PacketInfo::kNotAProbe));
+
   packet_router_->RemoveRtpModule(&rtp_1);
 
   // rtp_1 has been removed, try sending padding and make sure rtp_1 isn't asked
   // to send by not expecting any calls. Instead verify rtp_2 is called.
   EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true));
+  EXPECT_CALL(rtp_2, HasBweExtensions()).Times(1).WillOnce(Return(true));
   EXPECT_CALL(rtp_2, TimeToSendPadding(requested_padding_bytes, _)).Times(1);
   EXPECT_EQ(0u, packet_router_->TimeToSendPadding(requested_padding_bytes,
                                                   PacketInfo::kNotAProbe));

webrtc/modules/rtp_rtcp/include/rtp_rtcp.h

@@ -155,6 +155,8 @@ class RtpRtcp : public Module {
 
   virtual int32_t DeregisterSendRtpHeaderExtension(RTPExtensionType type) = 0;
 
+  virtual bool HasBweExtensions() const = 0;
+
   // Returns start timestamp.
   virtual uint32_t StartTimestamp() const = 0;
 

webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h

@@ -66,6 +66,7 @@ class MockRtpRtcp : public RtpRtcp {
                int32_t(RTPExtensionType type, uint8_t id));
   MOCK_METHOD1(DeregisterSendRtpHeaderExtension,
                int32_t(RTPExtensionType type));
+  MOCK_CONST_METHOD0(HasBweExtensions, bool());
   MOCK_CONST_METHOD0(StartTimestamp, uint32_t());
   MOCK_METHOD1(SetStartTimestamp, void(uint32_t timestamp));
   MOCK_CONST_METHOD0(SequenceNumber, uint16_t());

webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc

@@ -617,6 +617,15 @@ int32_t ModuleRtpRtcpImpl::DeregisterSendRtpHeaderExtension(
   return rtp_sender_.DeregisterRtpHeaderExtension(type);
 }
 
+bool ModuleRtpRtcpImpl::HasBweExtensions() const {
+  return rtp_sender_.IsRtpHeaderExtensionRegistered(
+             kRtpExtensionTransportSequenceNumber) ||
+         rtp_sender_.IsRtpHeaderExtensionRegistered(
+             kRtpExtensionAbsoluteSendTime) ||
+         rtp_sender_.IsRtpHeaderExtensionRegistered(
+             kRtpExtensionTransmissionTimeOffset);
+}
+
 // (TMMBR) Temporary Max Media Bit Rate.
 bool ModuleRtpRtcpImpl::TMMBR() const {
   return rtcp_sender_.TMMBR();

webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h

@@ -65,6 +65,8 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp {
 
   int32_t DeregisterSendRtpHeaderExtension(RTPExtensionType type) override;
 
+  bool HasBweExtensions() const override;
+
   // Get start timestamp.
   uint32_t StartTimestamp() const override;
 

webrtc/modules/rtp_rtcp/source/rtp_sender.cc

@@ -36,6 +36,7 @@ namespace webrtc {
 namespace {
 // Max in the RFC 3550 is 255 bytes, we limit it to be modulus 32 for SRTP.
 constexpr size_t kMaxPaddingLength = 224;
+constexpr size_t kMinAudioPaddingLength = 50;
 constexpr int kSendSideDelayWindowMs = 1000;
 constexpr size_t kRtpHeaderLength = 12;
 constexpr uint16_t kMaxInitRtpSeqNumber = 32767;  // 2^15 -1.
@@ -215,7 +216,7 @@ int32_t RTPSender::RegisterRtpHeaderExtension(RTPExtensionType type,
   return -1;
 }
 
-bool RTPSender::IsRtpHeaderExtensionRegistered(RTPExtensionType type) {
+bool RTPSender::IsRtpHeaderExtensionRegistered(RTPExtensionType type) const {
   rtc::CritScope lock(&send_critsect_);
   return rtp_header_extension_map_.IsRegistered(type);
 }
@@ -481,11 +482,20 @@ size_t RTPSender::TrySendRedundantPayloads(size_t bytes_to_send,
 }
 
 size_t RTPSender::SendPadData(size_t bytes, int probe_cluster_id) {
-  // Always send full padding packets. This is accounted for by the
+  size_t padding_bytes_in_packet;
-  // RtpPacketSender, which will make sure we don't send too much padding even
+  if (audio_configured_) {
-  // if a single packet is larger than requested.
+    // Allow smaller padding packets for audio.
-  size_t padding_bytes_in_packet =
+    padding_bytes_in_packet =
-      std::min(MaxPayloadSize(), kMaxPaddingLength);
+        std::min(std::max(bytes, kMinAudioPaddingLength), MaxPayloadSize());
+    if (padding_bytes_in_packet > kMaxPaddingLength)
+      padding_bytes_in_packet = kMaxPaddingLength;
+  } else {
+    // Always send full padding packets. This is accounted for by the
+    // RtpPacketSender, which will make sure we don't send too much padding even
+    // if a single packet is larger than requested.
+    // We do this to avoid frequently sending small packets on higher bitrates.
+    padding_bytes_in_packet = std::min(MaxPayloadSize(), kMaxPaddingLength);
+  }
   size_t bytes_sent = 0;
   while (bytes_sent < bytes) {
     int64_t now_ms = clock_->TimeInMilliseconds();
@@ -502,9 +512,15 @@ size_t RTPSender::SendPadData(size_t bytes, int probe_cluster_id) {
       timestamp = last_rtp_timestamp_;
       capture_time_ms = capture_time_ms_;
       if (rtx_ == kRtxOff) {
-        // Without RTX we can't send padding in the middle of frames.
+        if (payload_type_ == -1)
-        if (!last_packet_marker_bit_)
           break;
+        // Without RTX we can't send padding in the middle of frames.
+        // For audio marker bits doesn't mark the end of a frame and frames
+        // are usually a single packet, so for now we don't apply this rule
+        // for audio.
+        if (!audio_configured_ && !last_packet_marker_bit_) {
+          break;
+        }
         ssrc = ssrc_;
         sequence_number = sequence_number_;
         ++sequence_number_;
@@ -796,7 +812,7 @@ bool RTPSender::IsFecPacket(const RtpPacketToSend& packet) const {
 }
 
 size_t RTPSender::TimeToSendPadding(size_t bytes, int probe_cluster_id) {
-  if (audio_configured_ || bytes == 0)
+  if (bytes == 0)
     return 0;
   size_t bytes_sent = TrySendRedundantPayloads(bytes, probe_cluster_id);
   if (bytes_sent < bytes)

webrtc/modules/rtp_rtcp/source/rtp_sender.h

@@ -120,7 +120,7 @@ class RTPSender {
 
   // RTP header extension
   int32_t RegisterRtpHeaderExtension(RTPExtensionType type, uint8_t id);
-  bool IsRtpHeaderExtensionRegistered(RTPExtensionType type);
+  bool IsRtpHeaderExtensionRegistered(RTPExtensionType type) const;
   int32_t DeregisterRtpHeaderExtension(RTPExtensionType type);
 
   bool TimeToSendPacket(uint32_t ssrc,

webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc

@@ -1491,4 +1491,29 @@ TEST_F(RtpSenderTest, AddOverheadToTransportFeedbackObserver) {
   SendGenericPayload();
 }
 
+TEST_F(RtpSenderTest, SendAudioPadding) {
+  MockTransport transport;
+  const bool kEnableAudio = true;
+  rtp_sender_.reset(new RTPSender(
+      kEnableAudio, &fake_clock_, &transport, &mock_paced_sender_, nullptr,
+      nullptr, nullptr, nullptr, nullptr, nullptr, &mock_rtc_event_log_,
+      nullptr, &retransmission_rate_limiter_, nullptr));
+  rtp_sender_->SetSendPayloadType(kPayload);
+  rtp_sender_->SetSequenceNumber(kSeqNum);
+  rtp_sender_->SetTimestampOffset(0);
+  rtp_sender_->SetSSRC(kSsrc);
+
+  const size_t kPaddingSize = 59;
+  EXPECT_CALL(transport, SendRtp(_, kPaddingSize + kRtpHeaderSize, _))
+      .WillOnce(testing::Return(true));
+  EXPECT_EQ(kPaddingSize, rtp_sender_->TimeToSendPadding(
+                              kPaddingSize, PacketInfo::kNotAProbe));
+
+  // Requested padding size is too small, will send a larger one.
+  const size_t kMinPaddingSize = 50;
+  EXPECT_CALL(transport, SendRtp(_, kMinPaddingSize + kRtpHeaderSize, _))
+      .WillOnce(testing::Return(true));
+  EXPECT_EQ(kMinPaddingSize, rtp_sender_->TimeToSendPadding(
+                                 kMinPaddingSize - 5, PacketInfo::kNotAProbe));
+}
 }  // namespace webrtc