Wire up FlexfecSender in RTPSenderVideo.

webrtc/modules/rtp_rtcp/source/flexfec_sender.cc

 /*
  *  Copyright (c) 2016 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 20 matching lines @@
 // clock for the RTP timestamps. (This is according to the RFC, which states
 // that it is RECOMMENDED to use the same clock frequency for FlexFEC as for
 // the protected media stream.)
 // The constant converts from clock millisecond timestamps to the 90 kHz
 // RTP timestamp.
 const int kMsToRtpTimestamp = kVideoPayloadTypeFrequency / 1000;
 
 // How often to log the generated FEC packets to the text log.
 constexpr int64_t kPacketLogIntervalMs = 10000;
 
+// Only activate BWE header extensions for FlexFEC.
+RtpHeaderExtensionMap FilterRtpHeaderExtensions(
+    const std::vector<RtpExtension>& rtp_header_extensions) {
+  RtpHeaderExtensionMap map;
+  for (const auto& extension : rtp_header_extensions) {
+    if (extension.uri == RtpExtension::kTransportSequenceNumberUri) {
+      map.Register(kRtpExtensionTransportSequenceNumber, extension.id);

[danilchap, 2016/11/10 11:29:23]

I would like to remove this version of Register eventually,
can you write instead
if (extension.uri == TransportSequenceNumber::kUri) {
  map.Register<TransportSequenceNumber>(extension.id);
}
(template version also slightly faster, thought that is negligible)

[brandtr, 2016/11/10 12:00:41]

Done.

+    } else if (extension.uri == RtpExtension::kAbsSendTimeUri) {
+      map.Register(kRtpExtensionAbsoluteSendTime, extension.id);
+    } else if (extension.uri == RtpExtension::kTimestampOffsetUri) {
+      map.Register(kRtpExtensionTransmissionTimeOffset, extension.id);
+    } else {
+      LOG(LS_WARNING) << "RTP header extension with id: " << extension.id
+                      << ", uri: " << extension.uri
+                      << ", is unsupported by FlexfecSender.";
+    }
+  }
+  return map;
+}
+
 }  // namespace
 
 FlexfecSender::FlexfecSender(
     int payload_type,
     uint32_t ssrc,
     uint32_t protected_media_ssrc,
     const std::vector<RtpExtension>& rtp_header_extensions,
     Clock* clock)
     : clock_(clock),
       random_(clock_->TimeInMicroseconds()),
       last_generated_packet_ms_(-1),
       payload_type_(payload_type),
       // Initialize the timestamp offset and RTP sequence numbers randomly.
       // (This is not intended to be cryptographically strong.)
       timestamp_offset_(random_.Rand<uint32_t>()),
       ssrc_(ssrc),
       protected_media_ssrc_(protected_media_ssrc),
       seq_num_(random_.Rand(1, kMaxInitRtpSeqNumber)),
       ulpfec_generator_(ForwardErrorCorrection::CreateFlexfec()),
-      rtp_header_extension_map_() {
+      rtp_header_extension_map_(
+          FilterRtpHeaderExtensions(rtp_header_extensions)) {

[danilchap, 2016/11/10 11:29:23]

maybe  call this function 'RegisterBweExtensions'
so that it would fit on one line.

[brandtr, 2016/11/10 12:00:41]

Done.

   // This object should not have been instantiated if FlexFEC is disabled.
   RTC_DCHECK_GE(payload_type, 0);
   RTC_DCHECK_LE(payload_type, 127);
 
   // It's OK to create this object on a different thread/task queue than
   // the one used during main operation.
   sequence_checker_.Detach();
-
-  // Register RTP header extensions for BWE.
-  for (const auto& extension : rtp_header_extensions) {
-    if (extension.uri == RtpExtension::kTransportSequenceNumberUri) {
-      rtp_header_extension_map_.Register(kRtpExtensionTransportSequenceNumber,
-                                         extension.id);
-    } else if (extension.uri == RtpExtension::kAbsSendTimeUri) {
-      rtp_header_extension_map_.Register(kRtpExtensionAbsoluteSendTime,
-                                         extension.id);
-    } else if (extension.uri == RtpExtension::kTimestampOffsetUri) {
-      rtp_header_extension_map_.Register(kRtpExtensionTransmissionTimeOffset,
-                                         extension.id);
-    } else {
-      LOG(LS_WARNING) << "RTP header extension with id: " << extension.id
-                      << ", uri: " << extension.uri
-                      << ", is unsupported by FlexfecSender.";
-    }
-  }
 }
 
 FlexfecSender::~FlexfecSender() = default;
 
 // We are reusing the implementation from UlpfecGenerator for SetFecParameters,
 // AddRtpPacketAndGenerateFec, and FecAvailable.
 void FlexfecSender::SetFecParameters(const FecProtectionParams& params) {
   RTC_DCHECK_CALLED_SEQUENTIALLY(&sequence_checker_);
   ulpfec_generator_.SetFecParameters(params);
 }
@@ skipping 55 matching lines @@
       now_ms - last_generated_packet_ms_ > kPacketLogIntervalMs) {
     LOG(LS_INFO) << "Generated " << fec_packets_to_send.size()
                  << " FlexFEC packets with payload type: " << payload_type_
                  << " and SSRC: " << ssrc_ << ".";
     last_generated_packet_ms_ = now_ms;
   }
 
   return fec_packets_to_send;
 }
 
+// For FlexFEC, the overhead is the FEC header, as well as the BWE header exts.

[danilchap, 2016/11/10 11:29:23]

may be instead of shortering word remove redundant words:
// The overhead is BWE rtp header extensions and FlexFEC header.

[brandtr, 2016/11/10 12:00:41]

Done.

 size_t FlexfecSender::MaxPacketOverhead() const {
-  return kFlexfecMaxHeaderSize;
+  return kFlexfecMaxHeaderSize +
+         rtp_header_extension_map_.GetTotalLengthInBytes();

[danilchap, 2016/11/10 11:29:23]

really tiny nit: may be sum other way around since in a raw packet extensions go
before flexfec header.

[brandtr, 2016/11/10 12:00:41]

I like that! These things make the code easier to interpret, IMO. Thanks for the
comment.

 }
 
 }  // namespace webrtc