Fix timing frames and FEC conflict

webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc

Index: webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc
index 41af62b9faa3c6622d7d00086371b9a8c4774e33..9ce8032a7eb9f959088c2becc7838109e70f6ffa 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc
@@ -394,13 +394,20 @@ bool RTPSenderVideo::SendVideo(RtpVideoCodecTypes video_type,
     if (!rtp_sender_->AssignSequenceNumber(packet.get()))
       return false;
 
+    bool protect_packet =
+            (packetizer->GetProtectionType() == kProtectedPacket);
     // Put packetization finish timestamp into extension.
     if (last && is_timing_frame) {
       packet->set_packetization_finish_time_ms(clock_->TimeInMilliseconds());
+      // TODO(ilnik): Due to webrtc:7859, packets with timing extensions are not
+      // protected by FEC. It reduces FEC efficiency a bit. When FEC is moved
+      // below the pacer, it can be re-enabled for these packets.
+      // NOTE: Any RTP stream processor in the network, modifying 'network'
+      // timestamps in the timing frames extension have to be an end-point for
+      // FEC, otherwise recovered by FEC packets will be corrupted.
+      protect_packet = false;

[brandtr, 2017/06/21 11:05:39]

Can you add a unit test for this?

[ilnik, 2017/06/21 11:17:18]

What exactly do you want me to test? What packets with timing frames are not
protected by the FEC? What there are no corruptions with timing frames?

[brandtr, 2017/06/21 11:28:15]

The former. Either through a unit test here, or using an end-to-end test that
verifies that sent FEC packets are not protecting packets with timing headers. I
think the unit test should be less involved to write.

I will think about more ways to catch corruptions, so you don't have to add any
tests for that.

[ilnik, 2017/06/21 12:49:42]

Done.

     }
 
-    const bool protect_packet =
-        (packetizer->GetProtectionType() == kProtectedPacket);
     if (flexfec_enabled()) {
       // TODO(brandtr): Remove the FlexFEC code path when FlexfecSender
       // is wired up to PacedSender instead.