Make FlexFEC packets paceable through RTPSender.

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 23 matching lines @@
 
 namespace {
 // Max in the RFC 3550 is 255 bytes, we limit it to be modulus 32 for SRTP.
 constexpr size_t kMaxPaddingLength = 224;
 constexpr int kSendSideDelayWindowMs = 1000;
 constexpr size_t kRtpHeaderLength = 12;
 constexpr uint16_t kMaxInitRtpSeqNumber = 32767;  // 2^15 -1.
 constexpr uint32_t kTimestampTicksPerMs = 90;
 constexpr int kBitrateStatisticsWindowMs = 1000;
 
+constexpr size_t kFlexfecPacketsToStoreBeforePacing = 50;
+
 const char* FrameTypeToString(FrameType frame_type) {
   switch (frame_type) {
     case kEmptyFrame:
       return "empty";
     case kAudioFrameSpeech: return "audio_speech";
     case kAudioFrameCN: return "audio_cn";
     case kVideoFrameKey: return "video_key";
     case kVideoFrameDelta: return "video_delta";
   }
   return "";
@@ skipping 33 matching lines @@
       transport_sequence_number_allocator_(sequence_number_allocator),
       transport_feedback_observer_(transport_feedback_observer),
       last_capture_time_ms_sent_(0),
       transport_(transport),
       sending_media_(true),                      // Default to sending media.
       max_payload_length_(IP_PACKET_SIZE - 28),  // Default is IP-v4/UDP.
       payload_type_(-1),
       payload_type_map_(),
       rtp_header_extension_map_(),
       packet_history_(clock),
+      flexfec_packet_history_(clock),
       // Statistics
       rtp_stats_callback_(nullptr),
       total_bitrate_sent_(kBitrateStatisticsWindowMs,
                           RateStatistics::kBpsScale),
       nack_bitrate_sent_(kBitrateStatisticsWindowMs, RateStatistics::kBpsScale),
       frame_count_observer_(frame_count_observer),
       send_side_delay_observer_(send_side_delay_observer),
       event_log_(event_log),
       send_packet_observer_(send_packet_observer),
       bitrate_callback_(bitrate_callback),
@@ skipping 13 matching lines @@
   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);
+
+  // Store FlexFEC packets in the packet history data structure, so they can
+  // be found when paced.
+  if (flexfec_sender) {
+    flexfec_packet_history_.SetStorePacketsStatus(
+        true, kFlexfecPacketsToStoreBeforePacing);
+  }
 }
 
 RTPSender::~RTPSender() {
   // TODO(tommi): Use a thread checker to ensure the object is created and
   // deleted on the same thread.  At the moment this isn't possible due to
   // voe::ChannelOwner in voice engine.  To reproduce, run:
   // voe_auto_test --automated --gtest_filter=*MixManyChannelsForStressOpus
 
   // TODO(tommi,holmer): We don't grab locks in the dtor before accessing member
   // variables but we grab them in all other methods. (what's the design?)
@@ skipping 538 matching lines @@
     }
   }
 }
 
 void RTPSender::OnReceivedRtcpReportBlocks(
     const ReportBlockList& report_blocks) {
   playout_delay_oracle_.OnReceivedRtcpReportBlocks(report_blocks);
 }
 
 // Called from pacer when we can send the packet.
-bool RTPSender::TimeToSendPacket(uint16_t sequence_number,
+bool RTPSender::TimeToSendPacket(uint32_t ssrc,
+                                 uint16_t sequence_number,
                                  int64_t capture_time_ms,
                                  bool retransmission,
                                  int probe_cluster_id) {
-  std::unique_ptr<RtpPacketToSend> packet =
-      packet_history_.GetPacketAndSetSendTime(sequence_number, 0,
-                                              retransmission);
-  if (!packet) {
-    // Packet cannot be found. Allow sending to continue.
+  std::unique_ptr<RtpPacketToSend> packet;
+  if (ssrc == SSRC()) {
+    packet = packet_history_.GetPacketAndSetSendTime(sequence_number, 0,
+                                                     retransmission);
+  } else if (ssrc == FlexfecSsrc()) {
+    packet = flexfec_packet_history_.GetPacketAndSetSendTime(sequence_number, 0,
+                                                             retransmission);
+  } else {
+    // Incorrect SSRC. Allow sending to continue.

[danilchap, 2016/11/11 15:26:06]

because of if (!packet) return true; this block can be omitted,
but if you want to keep it, better RTC_NOTREACHABLE since checks this is right
module should be done outside.

[brandtr, 2016/11/14 09:48:09]

Right. Removing block.

     return true;
   }
 
+  if (!packet) {
+    // Packet cannot be found.

[stefan-webrtc, 2016/11/13 13:38:18]

Should we log a warning?

[brandtr, 2016/11/14 09:48:09]

That's already done within the packet history itself:
https://cs.chromium.org/chromium/src/third_party/webrtc/modules/rtp_rtcp/sour...

+    return true;
+  }
+
   return PrepareAndSendPacket(
       std::move(packet),
       retransmission && (RtxStatus() & kRtxRetransmitted) > 0, retransmission,
       probe_cluster_id);
 }
 
 bool RTPSender::PrepareAndSendPacket(std::unique_ptr<RtpPacketToSend> packet,
                                      bool send_over_rtx,
                                      bool is_retransmit,
                                      int probe_cluster_id) {
@@ skipping 119 matching lines @@
                                     FecOverheadRate() / 1000, packet->Ssrc());
     BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "VideoNackBitrate_kbps", now_ms,
                                     NackOverheadRate() / 1000, packet->Ssrc());
   } else {
     BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "AudioTotBitrate_kbps", now_ms,
                                     ActualSendBitrateKbit(), packet->Ssrc());
     BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "AudioNackBitrate_kbps", now_ms,
                                     NackOverheadRate() / 1000, packet->Ssrc());
   }
 
+  uint32_t ssrc = packet->Ssrc();
+  rtc::Optional<uint32_t> flexfec_ssrc = FlexfecSsrc();
   if (paced_sender_) {
     uint16_t seq_no = packet->SequenceNumber();
-    uint32_t ssrc = packet->Ssrc();
     // Correct offset between implementations of millisecond time stamps in
     // TickTime and Clock.
     int64_t corrected_time_ms = packet->capture_time_ms() + clock_delta_ms_;
     size_t payload_length = packet->payload_size();
-    packet_history_.PutRtpPacket(std::move(packet), storage, false);
+    if (ssrc == flexfec_ssrc) {
+      // Store FlexFEC packets in the history here, so they can be found
+      // when the pacer calls TimeToSendPacket.
+      flexfec_packet_history_.PutRtpPacket(std::move(packet), storage, false);
+    } else {
+      packet_history_.PutRtpPacket(std::move(packet), storage, false);
+    }
 
     paced_sender_->InsertPacket(priority, ssrc, seq_no, corrected_time_ms,
                                 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);
     }
@@ skipping 15 matching lines @@
   bool sent = SendPacketToNetwork(*packet, options);
 
   if (sent) {
     {
       rtc::CritScope lock(&send_critsect_);
       media_has_been_sent_ = true;
     }
     UpdateRtpStats(*packet, false, false);
   }
 
-  // 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
+  // There is no need to store FlexFEC packets in the history here, since we
+  // will never retransmit those.
+  if (ssrc == flexfec_ssrc)

[danilchap, 2016/11/11 15:26:06]

comment hint that instead you should check:
if (storage != kDontRetransmit) {
  RTC_DCHECK_EQ(ssrc, Ssrc());
  packet_histry_.Put();
}

return sent;

[brandtr, 2016/11/14 09:48:09]

This would change how we store the packets in the packet history though. Right
now, we insert the packets even if storage == kDontRetransmit. But then we never
return them from the packet history...
https://cs.chromium.org/chromium/src/third_party/webrtc/modules/rtp_rtcp/sour...
I'm not familiar enough with that data structure to know if this is reasonable
or not.

Just wanted to confirm that this is an intentional idea in your suggestion?

[danilchap, 2016/11/14 11:20:33]

I think it is reasonable (no point to store something you would never retrieve),
but on second though - doesn't belong to this CL, so just DCHECK(storage ==
kNoRetransmit) to be safer.

On third though - it is not even needed, if sender is configured in no pacer, no
nack mode, then history should be disabled altogether.

(Current behavior is not nice left over from my refactoring - before we stored
the packet before checking if pacer is enabled or not. I've split these two
cases, but didn't notice that in second case no need to store the packet all the
time).

+    return sent;
+
+  // Mark the media 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_.PutRtpPacket(std::move(packet), storage, true);
 
   return sent;
 }
 
 void RTPSender::UpdateDelayStatistics(int64_t capture_time_ms, int64_t now_ms) {
   if (!send_side_delay_observer_ || capture_time_ms <= 0)
     return;
 
@@ skipping 188 matching lines @@
   if (!sequence_number_forced_) {
     sequence_number_ = random_.Rand(1, kMaxInitRtpSeqNumber);
   }
 }
 
 uint32_t RTPSender::SSRC() const {
   rtc::CritScope lock(&send_critsect_);
   return ssrc_;
 }
 
+rtc::Optional<uint32_t> RTPSender::FlexfecSsrc() const {
+  if (video_) {
+    return video_->FlexfecSsrc();
+  }
+  return rtc::Optional<uint32_t>();
+}
+
 void RTPSender::SetCsrcs(const std::vector<uint32_t>& csrcs) {
   assert(csrcs.size() <= kRtpCsrcSize);
   rtc::CritScope lock(&send_critsect_);
   csrcs_ = csrcs;
 }
 
 void RTPSender::SetSequenceNumber(uint16_t seq) {
   rtc::CritScope lock(&send_critsect_);
   sequence_number_forced_ = true;
   sequence_number_ = seq;
@@ skipping 136 matching lines @@
   rtc::CritScope lock(&send_critsect_);
 
   RtpState state;
   state.sequence_number = sequence_number_rtx_;
   state.start_timestamp = timestamp_offset_;
 
   return state;
 }
 
 }  // namespace webrtc