Style updates to ProducerFec/FecReceiver.

webrtc/modules/rtp_rtcp/source/rtp_sender_video.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 95 matching lines @@
 
 void RTPSenderVideo::SendVideoPacketAsRed(uint8_t* data_buffer,
                                           const size_t payload_length,
                                           const size_t rtp_header_length,
                                           uint16_t media_seq_num,
                                           const uint32_t capture_timestamp,
                                           int64_t capture_time_ms,
                                           StorageType media_packet_storage,
                                           bool protect) {
   std::unique_ptr<RedPacket> red_packet;
-  std::vector<RedPacket*> fec_packets;
+  std::vector<std::unique_ptr<RedPacket>> fec_packets;
   StorageType fec_storage = kDontRetransmit;
   uint16_t next_fec_sequence_number = 0;
   {
     // Only protect while creating RED and FEC packets, not when sending.
     rtc::CritScope cs(&crit_);
-    red_packet.reset(producer_fec_.BuildRedPacket(
-        data_buffer, payload_length, rtp_header_length, red_payload_type_));
+    red_packet = ProducerFec::BuildRedPacket(
+        data_buffer, payload_length, rtp_header_length, red_payload_type_);
     if (protect) {
       producer_fec_.AddRtpPacketAndGenerateFec(data_buffer, payload_length,
                                                rtp_header_length);
     }
     uint16_t num_fec_packets = producer_fec_.NumAvailableFecPackets();
     if (num_fec_packets > 0) {
       next_fec_sequence_number =
           _rtpSender.AllocateSequenceNumber(num_fec_packets);
-      fec_packets = producer_fec_.GetFecPackets(
+      fec_packets = producer_fec_.GetFecPacketsAsRed(
           red_payload_type_, fec_payload_type_, next_fec_sequence_number,
           rtp_header_length);
       RTC_DCHECK_EQ(num_fec_packets, fec_packets.size());
       if (_retransmissionSettings & kRetransmitFECPackets)
         fec_storage = kAllowRetransmission;
     }
   }
   if (_rtpSender.SendToNetwork(
           red_packet->data(), red_packet->length() - rtp_header_length,
           rtp_header_length, capture_time_ms, media_packet_storage,
           RtpPacketSender::kLowPriority) == 0) {
     _videoBitrate.Update(red_packet->length());
     TRACE_EVENT_INSTANT2(TRACE_DISABLED_BY_DEFAULT("webrtc_rtp"),
                          "Video::PacketRed", "timestamp", capture_timestamp,
                          "seqnum", media_seq_num);
   } else {
     LOG(LS_WARNING) << "Failed to send RED packet " << media_seq_num;
   }
-  for (RedPacket* fec_packet : fec_packets) {
+  for (const auto& fec_packet : fec_packets) {
     if (_rtpSender.SendToNetwork(
             fec_packet->data(), fec_packet->length() - rtp_header_length,
             rtp_header_length, capture_time_ms, fec_storage,
             RtpPacketSender::kLowPriority) == 0) {
       _fecOverheadRate.Update(fec_packet->length());
       TRACE_EVENT_INSTANT2(TRACE_DISABLED_BY_DEFAULT("webrtc_rtp"),
                            "Video::PacketFec", "timestamp", capture_timestamp,
                            "seqnum", next_fec_sequence_number);
     } else {
       LOG(LS_WARNING) << "Failed to send FEC packet "
                       << next_fec_sequence_number;
     }
-    delete fec_packet;
     ++next_fec_sequence_number;
   }
+  fec_packets.clear();

[danilchap, 2016/07/19 14:55:32]

needed? (fec_packets is a local variable and this is end of the function)

[brandtr, 2016/07/21 09:03:58]

Good point :)

 }
 
 void RTPSenderVideo::SetGenericFECStatus(const bool enable,
                                          const uint8_t payloadTypeRED,
                                          const uint8_t payloadTypeFEC) {
   RTC_DCHECK(!enable || payloadTypeRED > 0);
   rtc::CritScope cs(&crit_);
   fec_enabled_ = enable;
   red_payload_type_ = payloadTypeRED;
   fec_payload_type_ = payloadTypeFEC;
@@ skipping 58 matching lines @@
       videoType, _rtpSender.MaxDataPayloadLength(),
       video_header ? &(video_header->codecHeader) : nullptr, frameType));
 
   StorageType storage;
   int red_payload_type;
   bool first_frame = first_frame_sent_();
   {
     rtc::CritScope cs(&crit_);
     FecProtectionParams* fec_params =
         frameType == kVideoFrameKey ? &key_fec_params_ : &delta_fec_params_;
-    producer_fec_.SetFecParameters(fec_params, 0);
+    // TODO(brandtr): Note that we currently do not use unequal protection in

[danilchap, 2016/07/19 14:55:32]

TODO hints that something should be done in the future. Comment doesn't describe
nor plan nor when that should be done.
Either rewrite comment with clear goal or turn it into normal comment, without
TODO.

[brandtr, 2016/07/21 09:03:58]

Done.

+    // the FEC. This is signalled both here (by setting the number of important
+    // packets to zero), as well as in ProducerFec::AddRtpPacketAndGenerateFec.
+    const int kNumImportantPackets = 0;

[danilchap, 2016/07/19 14:55:33]

might be constexpr.

[brandtr, 2016/07/21 09:03:58]

Done.

+    producer_fec_.SetFecParameters(fec_params, kNumImportantPackets);
     storage = packetizer->GetStorageType(_retransmissionSettings);
     red_payload_type = red_payload_type_;
   }
 
   // Register CVO rtp header extension at the first time when we receive a frame
   // with pending rotation.
   bool video_rotation_active = false;
   if (video_header && video_header->rotation != kVideoRotation_0) {
     video_rotation_active = _rtpSender.ActivateCVORtpHeaderExtension();
   }
@@ skipping 98 matching lines @@
   rtc::CritScope cs(&crit_);
   return _retransmissionSettings;
 }
 
 void RTPSenderVideo::SetSelectiveRetransmissions(uint8_t settings) {
   rtc::CritScope cs(&crit_);
   _retransmissionSettings = settings;
 }
 
 }  // namespace webrtc