Bug 1305813 - do not send empty StreamId;r?drno draft
authorNico Grunbaum
Thu, 22 Jun 2017 00:46:07 -0700
changeset 599233 601b52d27501d544ab95aa9fe7a505667cb659bc
parent 598817 ddddf4be5b1dc179ed231c0b2127a524cadd9787
child 634711 44b6572571f6a7a8316009fee148d93d0e6eb874
push id65455
push userna-g@nostrum.com
push dateThu, 22 Jun 2017 21:18:02 +0000
reviewersdrno
bugs1305813
milestone56.0a1
Bug 1305813 - do not send empty StreamId;r?drno Also moving RID (StreamID) storage to the stack to clear up the TODO item and simplify the code. When unset the StreamID is stored as an empty string. Added missing GUARDED_BY on rid_ Added a check to shortcut checking strlen on the StreamId when it is an empty string (which is most of the time). MozReview-Commit-ID: EPUlPNBXYsQ
media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_sender.h
media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc
--- a/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
@@ -92,17 +92,17 @@ RTPSender::RTPSender(
       transport_feedback_observer_(transport_feedback_observer),
       last_capture_time_ms_sent_(0),
       transport_(transport),
       sending_media_(true),                   // Default to sending media.
       max_packet_size_(IP_PACKET_SIZE - 28),  // Default is IP-v4/UDP.
       payload_type_(-1),
       payload_type_map_(),
       rtp_header_extension_map_(),
-      rid_(NULL),
+      rid_{0},
       packet_history_(clock),
       flexfec_packet_history_(clock),
       // Statistics
       rtp_stats_callback_(nullptr),
       total_bitrate_sent_(kBitrateStatisticsWindowMs,
                           RateStatistics::kBpsScale),
       nack_bitrate_sent_(kBitrateStatisticsWindowMs, RateStatistics::kBpsScale),
       frame_count_observer_(frame_count_observer),
@@ -161,20 +161,16 @@ RTPSender::~RTPSender() {
 
   SSRCDatabase::ReturnSSRCDatabase();
   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);
   }
-
-  if (rid_) {
-    delete[] rid_;
-  }
 }
 
 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);
 }
@@ -195,23 +191,22 @@ uint32_t RTPSender::FecOverheadRate() co
 
 uint32_t RTPSender::NackOverheadRate() const {
   rtc::CritScope cs(&statistics_crit_);
   return nack_bitrate_sent_.Rate(clock_->TimeInMilliseconds()).value_or(0);
 }
 
 int32_t RTPSender::SetRID(const char* rid) {
   rtc::CritScope lock(&send_critsect_);
-  // TODO(jesup) avoid allocations
-  if (!rid_ || strlen(rid_) < strlen(rid)) {
-    // rid rarely changes length....
-    delete [] rid_;
-    rid_ = new char[strlen(rid)+1];
+  const size_t len = rid ? strlen(rid) : 0;
+  if (!len || len >= sizeof(rid_)) {
+    rid_[0] = '\0';
+  } else {
+    memmove(&rid_[0], rid, len + 1);
   }
-  strcpy(rid_, rid);
   return 0;
 }
 
 int32_t RTPSender::RegisterRtpHeaderExtension(RTPExtensionType type,
                                               uint8_t id) {
   rtc::CritScope lock(&send_critsect_);
   switch (type) {
     case kRtpExtensionVideoRotation:
@@ -450,17 +445,17 @@ bool RTPSender::SendOutgoingData(FrameTy
     if (rtp_header) {
       playout_delay_oracle_.UpdateRequest(ssrc, rtp_header->playout_delay,
                                           sequence_number);
     }
 
     result = video_->SendVideo(video_type, frame_type, payload_type,
                                rtp_timestamp, capture_time_ms, payload_data,
                                payload_size, fragmentation, rtp_header,
-                               rid_);
+                               &rid_[0]);
   }
 
   rtc::CritScope cs(&statistics_crit_);
   // Note: This is currently only counting for video.
   if (frame_type == kVideoFrameKey) {
     ++frame_counts_.key_frames;
   } else if (frame_type == kVideoFrameDelta) {
     ++frame_counts_.delta_frames;
--- a/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_sender.h
+++ b/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_sender.h
@@ -275,17 +275,17 @@ class RTPSender {
 
   size_t max_packet_size_;
 
   int8_t payload_type_ GUARDED_BY(send_critsect_);
   std::map<int8_t, RtpUtility::Payload*> payload_type_map_;
 
   RtpHeaderExtensionMap rtp_header_extension_map_ GUARDED_BY(send_critsect_);
 
-  char* rid_;
+  char rid_[kRIDSize + 1] GUARDED_BY(send_critsect_);
 
   // Tracks the current request for playout delay limits from application
   // and decides whether the current RTP frame should include the playout
   // delay extension on header.
   PlayoutDelayOracle playout_delay_oracle_;
 
   RtpPacketHistory packet_history_;
   // TODO(brandtr): Remove |flexfec_packet_history_| when the FlexfecSender
--- a/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc
+++ b/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc
@@ -12,16 +12,17 @@
 
 #include <stdlib.h>
 #include <string.h>
 
 #include <memory>
 #include <vector>
 #include <utility>
 
+#include "webrtc/common_types.h"
 #include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/base/trace_event.h"
 #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "webrtc/modules/rtp_rtcp/source/byte_io.h"
 #include "webrtc/modules/rtp_rtcp/source/rtp_format_video_generic.h"
 #include "webrtc/modules/rtp_rtcp/source/rtp_format_vp8.h"
 #include "webrtc/modules/rtp_rtcp/source/rtp_format_vp9.h"
@@ -319,18 +320,21 @@ bool RTPSenderVideo::SendVideo(RtpVideoC
       // Set rotation when key frame or when changed (to follow standard).
       // Or when different from 0 (to follow current receiver implementation).
       VideoRotation current_rotation = video_header->rotation;
       if (frame_type == kVideoFrameKey || current_rotation != last_rotation_ ||
           current_rotation != kVideoRotation_0)
         rtp_header->SetExtension<VideoOrientation>(current_rotation);
       last_rotation_ = current_rotation;
     }
-    if (rid) {
-      rtp_header->SetExtensionWithLength<StreamId>(strlen(rid)-1, rid);
+    if (rid && rid[0]) {
+      const size_t len = strlen(rid);
+      if (len) {
+        rtp_header->SetExtensionWithLength<StreamId>(len - 1, rid);
+      }
     }
 
     // FEC settings.
     const FecProtectionParams& fec_params =
         frame_type == kVideoFrameKey ? key_fec_params_ : delta_fec_params_;
     if (flexfec_enabled())
       flexfec_sender_->SetFecParameters(fec_params);
     if (ulpfec_enabled())