Correcting the amount of padding when send side bwe includes RTP overhead.

webrtc/modules/rtp_rtcp/source/rtp_sender.cc

Index: webrtc/modules/rtp_rtcp/source/rtp_sender.cc
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
index 2fd8b3eff51b14efe27fe3f0c3660234a029a35b..ebf7a280f6f4396a3c026c53b3c02df4d9f2184e 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
@@ -455,33 +455,44 @@ size_t RTPSender::TrySendRedundantPayloads(size_t bytes_to_send,
   int bytes_left = static_cast<int>(bytes_to_send);
   while (bytes_left > 0) {
     std::unique_ptr<RtpPacketToSend> packet =
-        packet_history_.GetBestFittingPacket(bytes_left);
+        packet_history_.GetBestFittingPacket(bytes_left,
+                                             send_side_bwe_with_overhead_);
     if (!packet)
       break;
-    size_t payload_size = packet->payload_size();
+    // When WebRTC-SendSideBwe-WithOverhead is enabled, the padding budget
+    // includes overhead.
+    const size_t used_bytes =
+        send_side_bwe_with_overhead_
+            ? packet->size()
+            : packet->payload_size() - packet->headers_size();

[michaelt, 2017/03/29 14:45:21]

Why "payload_size - headers_size", i don't understand this part.

[minyue-webrtc, 2017/03/29 18:52:33]

neither do I understand it :) will fix. Thanks.

     if (!PrepareAndSendPacket(std::move(packet), true, false, pacing_info))
       break;
-    bytes_left -= payload_size;
+    bytes_left -= used_bytes;
   }
   return bytes_to_send - bytes_left;
 }
 
 size_t RTPSender::SendPadData(size_t bytes,
                               const PacedPacketInfo& pacing_info) {
-  size_t padding_bytes_in_packet;
+  // 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.
+  size_t padding_bytes_in_packet =
+      std::min(MaxPayloadSize(), kMaxPaddingLength);
+
   if (audio_configured_) {
     // Allow smaller padding packets for audio.
+
+    // When WebRTC-SendSideBwe-WithOverhead is enabled, the padding budget
+    // includes overhead.
+    const size_t padding_bytes_if_one_packet =
+        send_side_bwe_with_overhead_ ? bytes - RtpHeaderLength() : bytes;
     padding_bytes_in_packet =
-        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);
+        std::min(padding_bytes_in_packet,
+                 std::max(padding_bytes_if_one_packet, kMinAudioPaddingLength));
   }
+
   size_t bytes_sent = 0;
   while (bytes_sent < bytes) {

[michaelt, 2017/03/29 14:45:21]

if we define packet size before the loop. Will not the last packet be to big ?  

[minyue-webrtc, 2017/03/29 18:52:33]

I have not looked into this. will do

[danilchap, 2017/03/30 11:58:08]

Might not be important (for same reason padding might overshoot in video case):
it is better to send one big packet than two small, so it is ok to overshoot a
bit - next call to padding with request less of it.

[stefan-webrtc, 2017/03/30 12:09:03]

This is exactly why we do it this way. We want to avoid sending many small
packets and prefer temporary overshoots to ensure we have a lower packet rate.

     int64_t now_ms = clock_->TimeInMilliseconds();
@@ -566,6 +577,7 @@ size_t RTPSender::SendPadData(size_t bytes,
     PacketOptions options;
     bool has_transport_seq_num =
         UpdateTransportSequenceNumber(&padding_packet, &options.packet_id);
+
     padding_packet.SetPadding(padding_bytes_in_packet, &random_);
 
     if (has_transport_seq_num) {
@@ -576,7 +588,10 @@ size_t RTPSender::SendPadData(size_t bytes,
     if (!SendPacketToNetwork(padding_packet, options, pacing_info))
       break;
 
-    bytes_sent += padding_bytes_in_packet;
+    // When WebRTC-SendSideBwe-WithOverhead is enabled, the padding budget
+    // includes overhead.
+    bytes_sent += send_side_bwe_with_overhead_ ? padding_packet.padding_size()
+                                               : padding_packet.size();
     UpdateRtpStats(padding_packet, over_rtx, false);
   }