Add histogram stats for send delay for a sent video stream.

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 101 matching lines @@
 RTPSender::RTPSender(
     bool audio,
     Clock* clock,
     Transport* transport,
     RtpAudioFeedback* audio_feedback,
     RtpPacketSender* paced_sender,
     TransportSequenceNumberAllocator* sequence_number_allocator,
     TransportFeedbackObserver* transport_feedback_observer,
     BitrateStatisticsObserver* bitrate_callback,
     FrameCountObserver* frame_count_observer,
-    SendSideDelayObserver* send_side_delay_observer)
+    SendSideDelayObserver* send_side_delay_observer,
+    SendPacketObserver* send_packet_observer)
     : clock_(clock),
       // TODO(holmer): Remove this conversion when we remove the use of
       // TickTime.
       clock_delta_ms_(clock_->TimeInMilliseconds() -
                       TickTime::MillisecondTimestamp()),
       bitrates_(new BitrateAggregator(bitrate_callback)),
       total_bitrate_sent_(clock, bitrates_->total_bitrate_observer()),
       audio_configured_(audio),
       audio_(audio ? new RTPSenderAudio(clock, this, audio_feedback) : nullptr),
       video_(audio ? nullptr : new RTPSenderVideo(clock, this)),
@@ skipping 17 matching lines @@
       // NACK.
       nack_byte_count_times_(),
       nack_byte_count_(),
       nack_bitrate_(clock, bitrates_->retransmit_bitrate_observer()),
       packet_history_(clock),
       // Statistics
       statistics_crit_(CriticalSectionWrapper::CreateCriticalSection()),
       rtp_stats_callback_(NULL),
       frame_count_observer_(frame_count_observer),
       send_side_delay_observer_(send_side_delay_observer),
+      send_packet_observer_(send_packet_observer),
       // RTP variables
       start_timestamp_forced_(false),
       start_timestamp_(0),
       ssrc_db_(*SSRCDatabase::GetSSRCDatabase()),
       remote_ssrc_(0),
       sequence_number_forced_(false),
       ssrc_forced_(false),
       timestamp_(0),
       capture_time_ms_(0),
       last_timestamp_time_ms_(0),
@@ skipping 431 matching lines @@
                               bool timestamp_provided,
                               uint32_t timestamp,
                               int64_t capture_time_ms) {
   // 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.
   size_t padding_bytes_in_packet =
       std::min(MaxDataPayloadLength(), kMaxPaddingLength);
   size_t bytes_sent = 0;
-  bool using_transport_seq = rtp_header_extension_map_.IsRegistered(
-                                 kRtpExtensionTransportSequenceNumber) &&
-                             transport_sequence_number_allocator_;
   for (; bytes > 0; bytes -= padding_bytes_in_packet) {
     if (bytes < padding_bytes_in_packet)
       bytes = padding_bytes_in_packet;
 
     uint32_t ssrc;
     uint16_t sequence_number;
     int payload_type;
     bool over_rtx;
     {
       CriticalSectionScoped cs(send_critsect_.get());
@@ skipping 46 matching lines @@
     rtp_parser.Parse(rtp_header);
 
     if (capture_time_ms > 0) {
       UpdateTransmissionTimeOffset(
           padding_packet, length, rtp_header, now_ms - capture_time_ms);
     }
 
     UpdateAbsoluteSendTime(padding_packet, length, rtp_header, now_ms);
 
     PacketOptions options;
-    if (using_transport_seq) {
-      options.packet_id =
-          UpdateTransportSequenceNumber(padding_packet, length, rtp_header);
-    }
-
-    if (using_transport_seq && transport_feedback_observer_) {
-      transport_feedback_observer_->AddPacket(options.packet_id, length, true);
+    if (UseTransportSequenceNumber(&options.packet_id)) {
+      UpdateTransportSequenceNumber(options.packet_id, padding_packet, length,
+                                    rtp_header);
+      if (transport_feedback_observer_) {
+        transport_feedback_observer_->AddPacket(options.packet_id, length,
+                                                true);
+      }
     }
 
     if (!SendPacketToNetwork(padding_packet, length, options))
       break;
 
     bytes_sent += padding_bytes_in_packet;
     UpdateRtpStats(padding_packet, length, rtp_header, over_rtx, false);
   }
 
   return bytes_sent;
@@ skipping 181 matching lines @@
 
   if (!packet_history_.GetPacketAndSetSendTime(sequence_number,
                                                0,
                                                retransmission,
                                                data_buffer,
                                                &length,
                                                &stored_time_ms)) {
     // Packet cannot be found. Allow sending to continue.
     return true;
   }
-  if (!retransmission && capture_time_ms > 0) {
-    UpdateDelayStatistics(capture_time_ms, clock_->TimeInMilliseconds());
-  }
+
   int rtx;
   {
     CriticalSectionScoped lock(send_critsect_.get());
     rtx = rtx_;
   }
   return PrepareAndSendPacket(data_buffer,
                               length,
                               capture_time_ms,
                               retransmission && (rtx & kRtxRetransmitted) > 0,
                               retransmission);
@@ skipping 23 matching lines @@
     BuildRtxPacket(buffer, &length, data_buffer_rtx);
     buffer_to_send_ptr = data_buffer_rtx;
   }
 
   int64_t now_ms = clock_->TimeInMilliseconds();
   int64_t diff_ms = now_ms - capture_time_ms;
   UpdateTransmissionTimeOffset(buffer_to_send_ptr, length, rtp_header,
                                diff_ms);
   UpdateAbsoluteSendTime(buffer_to_send_ptr, length, rtp_header, now_ms);
 
-  // TODO(sprang): Potentially too much overhead in IsRegistered()?
-  bool using_transport_seq = rtp_header_extension_map_.IsRegistered(
-                                 kRtpExtensionTransportSequenceNumber) &&
-                             transport_sequence_number_allocator_;
-
   PacketOptions options;
-  if (using_transport_seq) {
-    options.packet_id =
-        UpdateTransportSequenceNumber(buffer_to_send_ptr, length, rtp_header);
+  if (UseTransportSequenceNumber(&options.packet_id)) {
+    UpdateTransportSequenceNumber(options.packet_id, buffer_to_send_ptr, length,
+                                  rtp_header);
+    if (transport_feedback_observer_)
+      transport_feedback_observer_->AddPacket(options.packet_id, length, true);
   }
 
-  if (using_transport_seq && transport_feedback_observer_) {
-    transport_feedback_observer_->AddPacket(options.packet_id, length, true);
+  if (!is_retransmit && !send_over_rtx) {
+    UpdateDelayStatistics(capture_time_ms, now_ms);
+    UpdateOnSendPacket(options.packet_id, capture_time_ms, rtp_header.ssrc);
   }
 
   bool ret = SendPacketToNetwork(buffer_to_send_ptr, length, options);
   if (ret) {
     CriticalSectionScoped lock(send_critsect_.get());
     media_has_been_sent_ = true;
   }
   UpdateRtpStats(buffer_to_send_ptr, length, rtp_header, send_over_rtx,
                  is_retransmit);
   return ret;
 }
 
+bool RTPSender::UseTransportSequenceNumber(int* packet_id) const {

[stefan-webrtc, 2016/01/18 19:48:25]

Maybe this should say that it actually allocates a sequence number even if the
extension isn't registered? I think it might be worth the extra code to split it
in two because of this. WDYT?

[åsapersson, 2016/04/06 14:52:37]

Changed to AllocateTransportSequenceNumber. Removed the registration check
(checked in UpdateTransportSequenceNumber).

+  if (!transport_sequence_number_allocator_)
+    return false;
+
+  *packet_id = transport_sequence_number_allocator_->AllocateSequenceNumber();
+
+  // TODO(sprang): Potentially too much overhead in IsRegistered()?
+  return rtp_header_extension_map_.IsRegistered(
+      kRtpExtensionTransportSequenceNumber);
+}
+
 void RTPSender::UpdateRtpStats(const uint8_t* buffer,
                                size_t packet_length,
                                const RTPHeader& header,
                                bool is_rtx,
                                bool is_retransmit) {
   StreamDataCounters* counters;
   // Get ssrc before taking statistics_crit_ to avoid possible deadlock.
   uint32_t ssrc = is_rtx ? RtxSsrc() : SSRC();
 
   CriticalSectionScoped lock(statistics_crit_.get());
@@ skipping 89 matching lines @@
                                 payload_length, false);
     if (last_capture_time_ms_sent_ == 0 ||
         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 0;
   }
-  if (capture_time_ms > 0) {
-    UpdateDelayStatistics(capture_time_ms, now_ms);
-  }
+
+  UpdateDelayStatistics(capture_time_ms, now_ms);
 
   size_t length = payload_length + rtp_header_length;
   bool sent = SendPacketToNetwork(buffer, length, PacketOptions());
 
   // Mark the packet as sent in the history even if send failed. Dropping a
   // packet here should be treated as any other packet drop so we should be
   // ready for a retransmission.
   packet_history_.SetSent(rtp_header.sequenceNumber);
 
   if (!sent)
     return -1;
 
   {
     CriticalSectionScoped lock(send_critsect_.get());
     media_has_been_sent_ = true;
   }
   UpdateRtpStats(buffer, length, rtp_header, false, false);
   return 0;
 }
 
 void RTPSender::UpdateDelayStatistics(int64_t capture_time_ms, int64_t now_ms) {
-  if (!send_side_delay_observer_)
+  if (!send_side_delay_observer_ || capture_time_ms <= 0)

[stefan-webrtc, 2016/01/18 19:48:25]

Do you think it would be possible to instead use a DCHECK:
DCHECK(capture_time_ms <= 0 || audio_configured_)?

[åsapersson, 2016/04/06 14:52:37]

Not sure if set in all codec wrappers, would need to check separately.

     return;
 
   uint32_t ssrc;
   int avg_delay_ms = 0;
   int max_delay_ms = 0;
   {
     CriticalSectionScoped lock(send_critsect_.get());
     ssrc = ssrc_;
   }
   {
@@ skipping 11 matching lines @@
       ++num_delays;
     }
     if (num_delays == 0)
       return;
     avg_delay_ms = (avg_delay_ms + num_delays / 2) / num_delays;
   }
   send_side_delay_observer_->SendSideDelayUpdated(avg_delay_ms, max_delay_ms,
                                                   ssrc);
 }
 
+void RTPSender::UpdateOnSendPacket(int packet_id,
+                                   int64_t capture_time_ms,
+                                   uint32_t ssrc) {
+  if (!send_packet_observer_ || capture_time_ms <= 0 || packet_id == -1)

[stefan-webrtc, 2016/01/18 19:48:25]

Same here about DCHECK. Should be able to DCHECK both on capture_time_ms as
above and also have one on packet_id >= 0 since it always should be set, right?

[åsapersson, 2016/04/06 14:52:37]

packet_id could be -1 as it is now.

+    return;
+
+  send_packet_observer_->OnSendPacket(packet_id, capture_time_ms, ssrc);
+}
+
 void RTPSender::ProcessBitrate() {
   CriticalSectionScoped cs(send_critsect_.get());
   total_bitrate_sent_.Process();
   nack_bitrate_.Process();
   if (audio_configured_) {
     return;
   }
   video_->ProcessBitrate();
 }
 
@@ skipping 470 matching lines @@
     default:
       RTC_NOTREACHED();
   }
 
   // Update absolute send time field (convert ms to 24-bit unsigned with 18 bit
   // fractional part).
   ByteWriter<uint32_t, 3>::WriteBigEndian(rtp_packet + offset + 1,
                                           ConvertMsTo24Bits(now_ms));
 }
 
-uint16_t RTPSender::UpdateTransportSequenceNumber(
+bool RTPSender::UpdateTransportSequenceNumber(
+    uint16_t sequence_number,
     uint8_t* rtp_packet,
     size_t rtp_packet_length,
     const RTPHeader& rtp_header) const {
   size_t offset;
   CriticalSectionScoped cs(send_critsect_.get());
 
   switch (VerifyExtension(kRtpExtensionTransportSequenceNumber, rtp_packet,
                           rtp_packet_length, rtp_header,
                           kTransportSequenceNumberLength, &offset)) {
     case ExtensionStatus::kNotRegistered:
-      return 0;
+      return false;
     case ExtensionStatus::kError:
       LOG(LS_WARNING) << "Failed to update transport sequence number";
-      return 0;
+      return false;
     case ExtensionStatus::kOk:
       break;
     default:
       RTC_NOTREACHED();
   }
 
-  uint16_t seq = transport_sequence_number_allocator_->AllocateSequenceNumber();
-  BuildTransportSequenceNumberExtension(rtp_packet + offset, seq);
-  return seq;
+  BuildTransportSequenceNumberExtension(rtp_packet + offset, sequence_number);
+  return true;
 }
 
 void RTPSender::SetSendingStatus(bool enabled) {
   if (enabled) {
     uint32_t frequency_hz = SendPayloadFrequency();
     uint32_t RTPtime = RtpUtility::GetCurrentRTP(clock_, frequency_hz);
 
     // Will be ignored if it's already configured via API.
     SetStartTimestamp(RTPtime, false);
   } else {
@@ skipping 257 matching lines @@
   CriticalSectionScoped lock(send_critsect_.get());
 
   RtpState state;
   state.sequence_number = sequence_number_rtx_;
   state.start_timestamp = start_timestamp_;
 
   return state;
 }
 
 }  // namespace webrtc