Fix for video protection_bitrate in BWE with overhead.

webrtc/video/video_send_stream.cc

Index: webrtc/video/video_send_stream.cc
diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc
index 9a3b51e278a0c4f69608e9ea5966f9b82e7f76d2..49fe0c696a43f9d4a32bea4d7aeb4afc08813cf9 100644
--- a/webrtc/video/video_send_stream.cc
+++ b/webrtc/video/video_send_stream.cc
@@ -276,6 +276,17 @@ int CalculateMaxPadBitrateBps(std::vector<VideoStream> streams,
   return pad_up_to_bitrate_bps;
 }
 
+uint32_t CalculateOverheadRateBps(uint32_t bitrate_bps,
+                                  size_t packet_size_byte,

[stefan-webrtc, 2016/12/12 16:19:55]

packet_size_bytes

[michaelt, 2016/12/13 13:07:16]

Done.

+                                  size_t overhead_byte_per_packet) {

[stefan-webrtc, 2016/12/12 16:19:55]

overhead_bytes_per_packet or packet_overhead_bytes?

[michaelt, 2016/12/13 13:07:16]

Done.

+  if (webrtc::field_trial::FindFullName("WebRTC-SendSideBwe-WithOverhead") !=
+      "Enabled")
+    return 0;
+  int packets_per_second = std::ceil(bitrate_bps / (8 * packet_size_byte));
+  return static_cast<uint32_t>(8 * overhead_byte_per_packet *
+                               packets_per_second);
+}
+
 }  // namespace
 
 namespace internal {
@@ -1186,17 +1197,13 @@ uint32_t VideoSendStreamImpl::OnBitrateUpdated(uint32_t bitrate_bps,
   RTC_DCHECK(payload_router_.IsActive())
       << "VideoSendStream::Start has not been called.";
 
-  if (webrtc::field_trial::FindFullName("WebRTC-SendSideBwe-WithOverhead") ==
-      "Enabled") {
-    // Substract overhead from bitrate.
-    rtc::CritScope lock(&overhead_bytes_per_packet_crit_);
-    int packets_per_second =
-        std::ceil(bitrate_bps / (8 * (config_->rtp.max_packet_size +
-                                      transport_overhead_bytes_per_packet_)));
-    uint32_t overhead_bps = static_cast<uint32_t>(
-        8 * overhead_bytes_per_packet_ * packets_per_second);
-    bitrate_bps = overhead_bps > bitrate_bps ? 0u : bitrate_bps - overhead_bps;
-  }
+  // Substract overhead from bitrate.
+  rtc::CritScope lock(&overhead_bytes_per_packet_crit_);
+  uint32_t overhead_bps = CalculateOverheadRateBps(
+      bitrate_bps,
+      config_->rtp.max_packet_size + transport_overhead_bytes_per_packet_,
+      overhead_bytes_per_packet_);
+  bitrate_bps = overhead_bps > bitrate_bps ? 0u : bitrate_bps - overhead_bps;
 
   // Get the encoder target rate. It is the estimated network rate -
   // protection overhead.
@@ -1208,7 +1215,13 @@ uint32_t VideoSendStreamImpl::OnBitrateUpdated(uint32_t bitrate_bps,
       std::min(encoder_max_bitrate_bps_, encoder_target_rate_bps_);
   vie_encoder_->OnBitrateUpdated(encoder_target_rate_bps_, fraction_loss, rtt);
   stats_proxy_->OnSetEncoderTargetRate(encoder_target_rate_bps_);
-  return protection_bitrate;
+  // Add overhead to the protection_bitrate
+  return protection_bitrate +
+         CalculateOverheadRateBps(protection_bitrate,
+                                  config_->rtp.max_packet_size +
+                                      transport_overhead_bytes_per_packet_ -
+                                      overhead_bytes_per_packet_,
+                                  overhead_bytes_per_packet_);

[stefan-webrtc, 2016/12/12 16:19:55]

The overhead for FEC isn't the same as the overhead for media packets. Maybe
it's not worth taking that into account here though...

[michaelt, 2016/12/13 08:07:09]

Do you think it's not worth to take the overhead into account or the differences
in overhead between FEC and media packets?

[stefan-webrtc, 2016/12/13 08:16:30]

The difference in overhead. Do you agree?

[michaelt, 2016/12/13 13:07:16]

I don't know how the video FEC packets are build. If you think the difference is
not big between FEC and media overhead, I'm OK with it.
 

[stefan-webrtc, 2016/12/13 13:26:04]

Instead of doing this, maybe we should do:

protection_bitrate = bitrate_bps -
CalculateOverheadRateBps(encoder_target_rate_bps_, ...);

and remove line 1206?

[michaelt, 2016/12/20 10:15:56]

Did something similar to your idea pleas take a look.

 }
 
 void VideoSendStreamImpl::EnableEncodedFrameRecording(