Add overhead per packet observer to the rtp_sender.

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 36915c53afeb7fdf9c3a18bf7de53ce4d7f67f84..19514d8c9e7660d549b1975e81dbcf5822a96aba 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
@@ -75,7 +75,8 @@ RTPSender::RTPSender(
     SendSideDelayObserver* send_side_delay_observer,
     RtcEventLog* event_log,
     SendPacketObserver* send_packet_observer,
-    RateLimiter* retransmission_rate_limiter)
+    RateLimiter* retransmission_rate_limiter,
+    OverheadSizeObserver* overhead_size_observer)
     : clock_(clock),
       // TODO(holmer): Remove this conversion?
       clock_delta_ms_(clock_->TimeInMilliseconds() - rtc::TimeMillis()),
@@ -116,7 +117,10 @@ RTPSender::RTPSender(
       last_packet_marker_bit_(false),
       csrcs_(),
       rtx_(kRtxOff),
-      retransmission_rate_limiter_(retransmission_rate_limiter) {
+      transport_overhead_bytes_per_packet_(0),
+      rtp_overhead_bytes_per_packet_(0),
+      retransmission_rate_limiter_(retransmission_rate_limiter),
+      overhead_size_observer_(overhead_size_observer) {
   ssrc_ = ssrc_db_->CreateSSRC();
   RTC_DCHECK(ssrc_ != 0);
   ssrc_rtx_ = ssrc_db_->CreateSSRC();
@@ -561,18 +565,15 @@ size_t RTPSender::DeprecatedSendPadData(size_t bytes,
           kTimestampTicksPerMs * (now_ms - capture_time_ms));
     }
     padding_packet.SetExtension<AbsoluteSendTime>(now_ms);
-
     PacketOptions options;
     bool has_transport_seq_no =
         UpdateTransportSequenceNumber(&padding_packet, &options.packet_id);
-
     padding_packet.SetPadding(padding_bytes_in_packet, &random_);
 
-    if (has_transport_seq_no && transport_feedback_observer_)
-      transport_feedback_observer_->AddPacket(
-          options.packet_id,
-          padding_packet.payload_size() + padding_packet.padding_size(),
-          probe_cluster_id);
+    if (has_transport_seq_no) {

[stefan-webrtc, 2016/11/15 09:59:47]

Could you please change seq_no to seq_num? Thanks

[michaelt, 2016/11/15 11:25:04]

Done.

+      AddPacketToTransportFeedback(options.packet_id, &padding_packet,
+                                   probe_cluster_id);
+    }
 
     if (!SendPacketToNetwork(padding_packet, options))
       break;
@@ -735,12 +736,9 @@ bool RTPSender::PrepareAndSendPacket(std::unique_ptr<RtpPacketToSend> packet,
   packet_to_send->SetExtension<AbsoluteSendTime>(now_ms);
 
   PacketOptions options;
-  if (UpdateTransportSequenceNumber(packet_to_send, &options.packet_id) &&
-      transport_feedback_observer_) {
-    transport_feedback_observer_->AddPacket(
-        options.packet_id,
-        packet_to_send->payload_size() + packet_to_send->padding_size(),
-        probe_cluster_id);
+  if (UpdateTransportSequenceNumber(packet_to_send, &options.packet_id)) {
+    AddPacketToTransportFeedback(options.packet_id, packet_to_send,
+                                 probe_cluster_id);
   }
 
   if (!is_retransmit && !send_over_rtx) {
@@ -858,11 +856,9 @@ bool RTPSender::SendToNetwork(std::unique_ptr<RtpPacketToSend> packet,
   }
 
   PacketOptions options;
-  if (UpdateTransportSequenceNumber(packet.get(), &options.packet_id) &&
-      transport_feedback_observer_) {
-    transport_feedback_observer_->AddPacket(
-        options.packet_id, packet->payload_size() + packet->padding_size(),
-        PacketInfo::kNotAProbe);
+  if (UpdateTransportSequenceNumber(packet.get(), &options.packet_id)) {
+    AddPacketToTransportFeedback(options.packet_id, packet.get(),
+                                 PacketInfo::kNotAProbe);
   }
 
   UpdateDelayStatistics(packet->capture_time_ms(), now_ms);
@@ -1244,4 +1240,32 @@ RtpState RTPSender::GetRtxRtpState() const {
   return state;
 }
 
+void RTPSender::SetTransportOverhead(int transport_overhead) {
+  rtc::CritScope lock(&send_critsect_);
+  if (overhead_size_observer_ &&
+      transport_overhead_bytes_per_packet_ !=
+          static_cast<size_t>(transport_overhead)) {
+    overhead_size_observer_->OnOverheadSizeChanged(
+        rtp_overhead_bytes_per_packet_ + transport_overhead);
+  }
+  transport_overhead_bytes_per_packet_ = transport_overhead;
+}
+
+void RTPSender::AddPacketToTransportFeedback(uint16_t packet_id,
+                                             RtpPacketToSend* packet,

[stefan-webrtc, 2016/11/15 09:59:47]

Should be possible to pass in a const RtpPacketToSend&, right?

[michaelt, 2016/11/15 11:25:04]

Right.

+                                             int probe_cluster_id) {
+  rtc::CritScope lock(&send_critsect_);
+  if (transport_feedback_observer_) {
+    transport_feedback_observer_->AddPacket(
+        packet_id, packet->payload_size() + packet->padding_size(),
+        probe_cluster_id);
+  }
+  if (overhead_size_observer_ &&
+      rtp_overhead_bytes_per_packet_ != packet->headers_size()) {

[stefan-webrtc, 2016/11/15 09:59:47]

Why do we report this only if we have transport sequence numbers? Seems like
it's independent.

[michaelt, 2016/11/15 11:25:04]

I general you are right. Will decouple overhead_size from the transport sequence
numbers at this place. We just have to keep in mind that the overhead reduction
just have to be done in the case we use TWCC.  

+    overhead_size_observer_->OnOverheadSizeChanged(

[stefan-webrtc, 2016/11/15 09:59:47]

We shouldn't hold the send_critsect_ when calling this.

Btw, I'm curious what you think of using a getter for this instead of an
observer interface? Seems like it would potentially reduce the code needed. Or
do we need to know the overhead for every packet? That is a bit hard to judge
without seeing how you're going to use this.

[michaelt, 2016/11/15 11:25:04]

Changed the impl. so that OnOverheadSizeChanged is not called when we hold
send_critsect_. 

We don't need to know the overhead for each packet. 
Since the ANA will not know when the Overhead will change it has to ask for
every calculation of the payload bit rate. Which will be quite a lot during the
call. While o the other-hand the overhead should be stable during a call and
OnOverheadSizeChanged should just be called on the call start and on route
changes. So i think the observer is the better solution for the problem.
	

+        packet->headers_size() + transport_overhead_bytes_per_packet_);
+  }
+  rtp_overhead_bytes_per_packet_ = packet->headers_size();
+}
+
 }  // namespace webrtc