Fix timing frames and FEC conflict

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 718 matching lines @@
                        "seqnum", packet->SequenceNumber());
 
   std::unique_ptr<RtpPacketToSend> packet_rtx;
   if (send_over_rtx) {
     packet_rtx = BuildRtxPacket(*packet);
     if (!packet_rtx)
       return false;
     packet_to_send = packet_rtx.get();
   }
 
+  // Bug webrtc:7859. While FEC is invoked from rtp_sender_video, and not after
+  // the pacer, these modifications of the header below are happening after the
+  // FEC protection packets are calculated. This will corrupt recovered packets
+  // at the same place. It's not an issue for extensions, which are present in
+  // all the packets (their content just may be incorrect on recovered packets).
+  // In case of VideoTimingExtension, since it's present not in every packet,
+  // data after rtp header may be corrupted if these packets are protected by
+  // the FEC.
   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>(
       AbsoluteSendTime::MsTo24Bits(now_ms));
 
-  // TODO(ilnik): (webrtc:7859) For now we can't modify pacer exit timestamp in
-  // video timing extension because only some packets have it and it will break
-  // FEC recovered packets, which will lead to corruptions. Ideally, here
-  // |packet->set_pacer_exit_time_ms(now_ms)| should be called if
-  // |VideoTimingExtension| is present.
+  if (packet_to_send->HasExtension<VideoTimingExtension>())
+    packet_to_send->set_pacer_exit_time_ms(now_ms);
 
   PacketOptions options;
   if (UpdateTransportSequenceNumber(packet_to_send, &options.packet_id)) {
     AddPacketToTransportFeedback(options.packet_id, *packet_to_send,
                                  pacing_info);
   }
 
   if (!is_retransmit && !send_over_rtx) {
     UpdateDelayStatistics(packet->capture_time_ms(), now_ms);
     UpdateOnSendPacket(options.packet_id, packet->capture_time_ms(),
@@ skipping 68 matching lines @@
                               RtpPacketSender::Priority priority) {
   RTC_DCHECK(packet);
   int64_t now_ms = clock_->TimeInMilliseconds();
 
   // |capture_time_ms| <= 0 is considered invalid.
   // TODO(holmer): This should be changed all over Video Engine so that negative
   // time is consider invalid, while 0 is considered a valid time.
   if (packet->capture_time_ms() > 0) {
     packet->SetExtension<TransmissionOffset>(
         kTimestampTicksPerMs * (now_ms - packet->capture_time_ms()));
-    // TODO(ilnik): (webrtc:7859) For now we can't modify pacer exit timestamp
-    // in video timing extension because only some packets have it an it will
-    // break FEC recovered packets, which will lead to corruptions. Ideally,
-    // here |packet->set_pacer_exit_time_ms(now_ms)| should be called if
-    // |VideoTimingExtension| is present.
+    if (packet->HasExtension<VideoTimingExtension>())
+      packet->set_pacer_exit_time_ms(now_ms);
   }
   packet->SetExtension<AbsoluteSendTime>(AbsoluteSendTime::MsTo24Bits(now_ms));
 
   if (video_) {
     BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "VideoTotBitrate_kbps", now_ms,
                                     ActualSendBitrateKbit(), packet->Ssrc());
     BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "VideoFecBitrate_kbps", now_ms,
                                     FecOverheadRate() / 1000, packet->Ssrc());
     BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "VideoNackBitrate_kbps", now_ms,
                                     NackOverheadRate() / 1000, packet->Ssrc());
@@ skipping 427 matching lines @@
     if (rtp_overhead_bytes_per_packet_ == packet.headers_size()) {
       return;
     }
     rtp_overhead_bytes_per_packet_ = packet.headers_size();
     overhead_bytes_per_packet = rtp_overhead_bytes_per_packet_;
   }
   overhead_observer_->OnOverheadChanged(overhead_bytes_per_packet);
 }
 
 }  // namespace webrtc