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
--- 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())