Add overhead per packet observer to the rtp_sender.

webrtc/modules/rtp_rtcp/source/rtp_sender.cc

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 57 matching lines @@
     Transport* transport,
     RtpPacketSender* paced_sender,
     FlexfecSender* flexfec_sender,
     TransportSequenceNumberAllocator* sequence_number_allocator,
     TransportFeedbackObserver* transport_feedback_observer,
     BitrateStatisticsObserver* bitrate_callback,
     FrameCountObserver* frame_count_observer,
     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()),
       random_(clock_->TimeInMicroseconds()),
       audio_configured_(audio),
       audio_(audio ? new RTPSenderAudio(clock, this) : nullptr),
       video_(audio ? nullptr : new RTPSenderVideo(clock, this, flexfec_sender)),
       paced_sender_(paced_sender),
       transport_sequence_number_allocator_(sequence_number_allocator),
       transport_feedback_observer_(transport_feedback_observer),
@@ skipping 20 matching lines @@
       remote_ssrc_(0),
       sequence_number_forced_(false),
       ssrc_forced_(false),
       last_rtp_timestamp_(0),
       capture_time_ms_(0),
       last_timestamp_time_ms_(0),
       media_has_been_sent_(false),
       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();
   RTC_DCHECK(ssrc_rtx_ != 0);
 
   // This random initialization is not intended to be cryptographic strong.
   timestamp_offset_ = random_.Rand<uint32_t>();
   // Random start, 16 bits. Can't be 0.
   sequence_number_rtx_ = random_.Rand(1, kMaxInitRtpSeqNumber);
   sequence_number_ = random_.Rand(1, kMaxInitRtpSeqNumber);
@@ skipping 424 matching lines @@
     padding_packet.SetTimestamp(timestamp);
     padding_packet.SetSsrc(ssrc);
 
     int64_t now_ms = clock_->TimeInMilliseconds();
 
     if (capture_time_ms > 0) {
       padding_packet.SetExtension<TransmissionOffset>(
           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;
 
     bytes_sent += padding_bytes_in_packet;
     UpdateRtpStats(padding_packet, over_rtx, false);
   }
 
   return bytes_sent;
 }
@@ skipping 142 matching lines @@
     packet_to_send = packet_rtx.get();
   }
 
   int64_t now_ms = clock_->TimeInMilliseconds();
   int64_t diff_ms = now_ms - capture_time_ms;
   packet_to_send->SetExtension<TransmissionOffset>(kTimestampTicksPerMs *
                                                    diff_ms);
   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) {
     UpdateDelayStatistics(packet->capture_time_ms(), now_ms);
     UpdateOnSendPacket(options.packet_id, packet->capture_time_ms(),
                        packet->Ssrc());
   }
 
   if (!SendPacketToNetwork(*packet_to_send, options))
     return false;
@@ skipping 97 matching lines @@
         corrected_time_ms > last_capture_time_ms_sent_) {
       last_capture_time_ms_sent_ = corrected_time_ms;
       TRACE_EVENT_ASYNC_BEGIN1(TRACE_DISABLED_BY_DEFAULT("webrtc_rtp"),
                                "PacedSend", corrected_time_ms,
                                "capture_time_ms", corrected_time_ms);
     }
     return true;
   }
 
   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);
   UpdateOnSendPacket(options.packet_id, packet->capture_time_ms(),
                      packet->Ssrc());
 
   bool sent = SendPacketToNetwork(*packet, options);
 
   if (sent) {
     {
@@ skipping 361 matching lines @@
 RtpState RTPSender::GetRtxRtpState() const {
   rtc::CritScope lock(&send_critsect_);
 
   RtpState state;
   state.sequence_number = sequence_number_rtx_;
   state.start_timestamp = timestamp_offset_;
 
   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