Remove hardcoded kValueSizeBytes values from variable-length header extensions.

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 27 matching lines @@
 constexpr size_t kMaxPaddingLength = 224;
 constexpr size_t kMinAudioPaddingLength = 50;
 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 kMinFlexfecPacketsToStoreForPacing = 50;
 
+template <typename Extension>
+constexpr RtpExtensionSize CreateExtensionSize() {
+  return {Extension::kId, Extension::kValueSizeBytes};
+}
+
+// Size info for header extensions that might be used in padding or FEC packets.
+constexpr RtpExtensionSize kExtensionSizes[] = {
+    CreateExtensionSize<AbsoluteSendTime>(),
+    CreateExtensionSize<TransmissionOffset>(),
+    CreateExtensionSize<TransportSequenceNumber>(),
+    CreateExtensionSize<PlayoutDelayLimits>(),
+};
+
 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 92 matching lines @@
   // Start documenting what thread we're on in what method so that it's easier
   // to understand performance attributes and possibly remove locks.
   while (!payload_type_map_.empty()) {
     std::map<int8_t, RtpUtility::Payload*>::iterator it =
         payload_type_map_.begin();
     delete it->second;
     payload_type_map_.erase(it);
   }
 }
 
+rtc::ArrayView<const RtpExtensionSize> RTPSender::FecExtensionSizes() {
+  return rtc::ArrayView<const RtpExtensionSize>(kExtensionSizes,
+                                                sizeof(kExtensionSizes));

[danilchap, 2017/05/10 13:27:13]

sizeof would return size in bytes, but array view need number of elements
you might want to use arraysize macro from "webrtc/base/arraysize.h"

Before that add a test to make it fail.

[erikvarga1, 2017/05/10 15:39:31]

Oh right, my bad. Fixed now. I've added a test to see how MaxPacketOverhead
works when there are header extensions (and it fails when sizeof is used here).

+}
+
 uint16_t RTPSender::ActualSendBitrateKbit() const {
   rtc::CritScope cs(&statistics_crit_);
   return static_cast<uint16_t>(
       total_bitrate_sent_.Rate(clock_->TimeInMilliseconds()).value_or(0) /
       1000);
 }
 
 uint32_t RTPSender::VideoBitrateSent() const {
   if (video_) {
     return video_->VideoBitrateSent();
@@ skipping 100 matching lines @@
 }
 
 void RTPSender::SetMaxRtpPacketSize(size_t max_packet_size) {
   // Sanity check.
   RTC_DCHECK(max_packet_size >= 100 && max_packet_size <= IP_PACKET_SIZE)
       << "Invalid max payload length: " << max_packet_size;
   rtc::CritScope lock(&send_critsect_);
   max_packet_size_ = max_packet_size;
 }
 
-size_t RTPSender::MaxPayloadSize() const {
-  if (audio_configured_) {
-    return max_packet_size_ - RtpHeaderLength();
-  } else {
-    return max_packet_size_ - RtpHeaderLength()   // RTP overhead.
-           - video_->FecPacketOverhead()          // FEC/ULP/RED overhead.
-           - (RtxStatus() ? kRtxHeaderSize : 0);  // RTX overhead.
-  }
-}
-
 size_t RTPSender::MaxRtpPacketSize() const {
   return max_packet_size_;
 }
 
 void RTPSender::SetRtxStatus(int mode) {
   rtc::CritScope lock(&send_critsect_);
   rtx_ = mode;
 }
 
 int RTPSender::RtxStatus() const {
@@ skipping 149 matching lines @@
     if (!PrepareAndSendPacket(std::move(packet), true, false, pacing_info))
       break;
     bytes_left -= payload_size;
   }
   return bytes_to_send - bytes_left;
 }
 
 size_t RTPSender::SendPadData(size_t bytes,
                               const PacedPacketInfo& pacing_info) {
   size_t padding_bytes_in_packet;
+
   if (audio_configured_) {
     // Allow smaller padding packets for audio.
+    size_t max_payload_size = max_packet_size_ - RtpHeaderLength();
     padding_bytes_in_packet =
-        std::min(std::max(bytes, kMinAudioPaddingLength), MaxPayloadSize());
+        std::min(std::max(bytes, kMinAudioPaddingLength), max_payload_size);
     if (padding_bytes_in_packet > kMaxPaddingLength)
       padding_bytes_in_packet = kMaxPaddingLength;
   } else {
     // Always send full padding packets. This is accounted for by the
     // RtpPacketSender, which will make sure we don't send too much padding even
     // if a single packet is larger than requested.
     // We do this to avoid frequently sending small packets on higher bitrates.
-    padding_bytes_in_packet = std::min(MaxPayloadSize(), kMaxPaddingLength);
+    size_t max_payload_size =
+        max_packet_size_ - RtpHeaderLength()   // RTP overhead.
+        - video_->FecPacketOverhead()          // FEC/ULP/RED overhead.
+        - (RtxStatus() ? kRtxHeaderSize : 0);  // RTX overhead.
+    padding_bytes_in_packet = std::min(max_payload_size, kMaxPaddingLength);
   }
   size_t bytes_sent = 0;
   while (bytes_sent < bytes) {
     int64_t now_ms = clock_->TimeInMilliseconds();
     uint32_t ssrc;
     uint32_t timestamp;
     int64_t capture_time_ms;
     uint16_t sequence_number;
     int payload_type;
     bool over_rtx;
@@ skipping 473 matching lines @@
 
   rtc::CritScope lock(&statistics_crit_);
   bitrate_callback_->Notify(total_bitrate_sent_.Rate(now_ms).value_or(0),
                             nack_bitrate_sent_.Rate(now_ms).value_or(0), ssrc);
 }
 
 size_t RTPSender::RtpHeaderLength() const {
   rtc::CritScope lock(&send_critsect_);
   size_t rtp_header_length = kRtpHeaderLength;
   rtp_header_length += sizeof(uint32_t) * csrcs_.size();
-  rtp_header_length += rtp_header_extension_map_.GetTotalLengthInBytes();
+  rtp_header_length +=
+      rtp_header_extension_map_.GetTotalLengthInBytes(kExtensionSizes);
   return rtp_header_length;
 }
 
 uint16_t RTPSender::AllocateSequenceNumber(uint16_t packets_to_send) {
   rtc::CritScope lock(&send_critsect_);
   uint16_t first_allocated_sequence_number = sequence_number_;
   sequence_number_ += packets_to_send;
   return first_allocated_sequence_number;
 }
 
@@ skipping 282 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