Make OverheadObserver::OnOverheadChanged count RTP headers only

webrtc/voice_engine/channel.cc

Index: webrtc/voice_engine/channel.cc
diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc
index 100832de3adeb2dc986cd255dc271a109ea6bbb7..14c63538279670d5502de2244552bb00d1d63a25 100644
--- a/webrtc/voice_engine/channel.cc
+++ b/webrtc/voice_engine/channel.cc
@@ -914,6 +914,8 @@ Channel::Channel(int32_t channelId,
       _lastLocalTimeStamp(0),
       _lastPayloadType(0),
       _includeAudioLevelIndication(false),
+      _transport_overhead_per_packet(0),
+      _rtp_overhead_per_packet(0),
       _outputSpeechType(AudioFrame::kNormalSpeech),
       restored_packet_in_use_(false),
       rtcp_observer_(new VoERtcpObserver(this)),
@@ -2878,18 +2880,26 @@ void Channel::SetRtcpRttStats(RtcpRttStats* rtcp_rtt_stats) {
   rtcp_rtt_stats_proxy_->SetRtcpRttStats(rtcp_rtt_stats);
 }
 
-void Channel::SetTransportOverhead(int transport_overhead_per_packet) {
-  _rtpRtcpModule->SetTransportOverhead(transport_overhead_per_packet);
-}
-
-void Channel::OnOverheadChanged(size_t overhead_bytes_per_packet) {
+void Channel::UpdateOverheadForEncoder() {
   audio_coding_->ModifyEncoder([&](std::unique_ptr<AudioEncoder>* encoder) {
     if (*encoder) {
-      (*encoder)->OnReceivedOverhead(overhead_bytes_per_packet);
+      (*encoder)->OnReceivedOverhead(
+          _transport_overhead_per_packet + _rtp_overhead_per_packet);
     }
   });
 }
 
+void Channel::SetTransportOverhead(int transport_overhead_per_packet) {
+  _transport_overhead_per_packet = transport_overhead_per_packet;
+  UpdateOverheadForEncoder();
+}
+
+
+void Channel::OnOverheadChanged(size_t overhead_bytes_per_packet) {

[the sun, 2016/12/21 10:33:02]

Not strictly part of this CL, but... "transport_overhead_per_packet" and
"overhead_bytes_per_packet" seem to me, judging from their names, that they
would overlap. We're talking about overhead for UDP/RTP per packet? Wouldn't
this be constant and therefore just amount to an offset in the final estimate?
Is our BWE really so accurate that we need this information? And coming from two
different sources at that? I can see that would at best contribute little, and
at worst cause racing bugs...

stefan, michaelt?

[stefan-webrtc, 2016/12/21 11:31:43]

It's important for really low bitrates where the overhead is significant. Take
for instance opus at 6 kbps, with 20 ms frames we have about 15 payload bytes
per packet, while with 100 ms frames we have 75 bytes per packet.

The rtp overhead (around 20 bytes) will amount to 133% in the former case and
only 27% in the latter case. Switching frame lengths may therefore be detected
as a link capacity change by the BWE if we don't include headers.

+  _rtp_overhead_per_packet = overhead_bytes_per_packet;
+  UpdateOverheadForEncoder();
+}
+
 int Channel::RegisterExternalMediaProcessing(ProcessingTypes type,
                                              VoEMediaProcess& processObject) {
   WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId),