 Chromium Code Reviews
 Chromium Code Reviews Issue 1623543002:
  Refactor RtpSender and SSRCDatabase a bit.  (Closed) 
  Base URL: https://chromium.googlesource.com/external/webrtc.git@master
    
  
    Issue 1623543002:
  Refactor RtpSender and SSRCDatabase a bit.  (Closed) 
  Base URL: https://chromium.googlesource.com/external/webrtc.git@master| Index: webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 
| diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 
| index bd9314d5d768b6e2d30c5be2a4ef2287b9c35ef5..0a83944e45d1bffc8c967d5d8130b584c7119c8c 100644 | 
| --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 
| +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 
| @@ -63,56 +63,46 @@ uint32_t ConvertMsTo24Bits(int64_t time_ms) { | 
| } | 
| } // namespace | 
| -class BitrateAggregator { | 
| - public: | 
| - explicit BitrateAggregator(BitrateStatisticsObserver* bitrate_callback) | 
| - : callback_(bitrate_callback), | 
| - total_bitrate_observer_(*this), | 
| - retransmit_bitrate_observer_(*this), | 
| - ssrc_(0) {} | 
| - | 
| - void OnStatsUpdated() const { | 
| - if (callback_) | 
| - callback_->Notify(total_bitrate_observer_.statistics(), | 
| - retransmit_bitrate_observer_.statistics(), | 
| - ssrc_); | 
| - } | 
| - | 
| - Bitrate::Observer* total_bitrate_observer() { | 
| - return &total_bitrate_observer_; | 
| - } | 
| - Bitrate::Observer* retransmit_bitrate_observer() { | 
| - return &retransmit_bitrate_observer_; | 
| - } | 
| - | 
| - void set_ssrc(uint32_t ssrc) { ssrc_ = ssrc; } | 
| - | 
| - private: | 
| - // We assume that these observers are called on the same thread, which is | 
| - // true for RtpSender as they are called on the Process thread. | 
| - class BitrateObserver : public Bitrate::Observer { | 
| - public: | 
| - explicit BitrateObserver(const BitrateAggregator& aggregator) | 
| - : aggregator_(aggregator) {} | 
| - | 
| - // Implements Bitrate::Observer. | 
| - void BitrateUpdated(const BitrateStatistics& stats) override { | 
| - statistics_ = stats; | 
| - aggregator_.OnStatsUpdated(); | 
| - } | 
| +RTPSender::BitrateAggregator::BitrateAggregator( | 
| + BitrateStatisticsObserver* bitrate_callback) | 
| + : callback_(bitrate_callback), | 
| + total_bitrate_observer_(*this), | 
| + retransmit_bitrate_observer_(*this), | 
| + ssrc_(0) {} | 
| - BitrateStatistics statistics() const { return statistics_; } | 
| +void RTPSender::BitrateAggregator::OnStatsUpdated() const { | 
| + if (callback_) { | 
| + callback_->Notify(total_bitrate_observer_.statistics(), | 
| + retransmit_bitrate_observer_.statistics(), ssrc_); | 
| + } | 
| +} | 
| - private: | 
| - BitrateStatistics statistics_; | 
| - const BitrateAggregator& aggregator_; | 
| - }; | 
| +Bitrate::Observer* RTPSender::BitrateAggregator::total_bitrate_observer() { | 
| + return &total_bitrate_observer_; | 
| +} | 
| +Bitrate::Observer* RTPSender::BitrateAggregator::retransmit_bitrate_observer() { | 
| + return &retransmit_bitrate_observer_; | 
| +} | 
| - BitrateStatisticsObserver* const callback_; | 
| - BitrateObserver total_bitrate_observer_; | 
| - BitrateObserver retransmit_bitrate_observer_; | 
| - uint32_t ssrc_; | 
| -}; | 
| +void RTPSender::BitrateAggregator::set_ssrc(uint32_t ssrc) { | 
| + ssrc_ = ssrc; | 
| +} | 
| + | 
| +RTPSender::BitrateAggregator::BitrateObserver::BitrateObserver( | 
| + const BitrateAggregator& aggregator) | 
| + : aggregator_(aggregator) {} | 
| + | 
| +// Implements Bitrate::Observer. | 
| +void RTPSender::BitrateAggregator::BitrateObserver::BitrateUpdated( | 
| + const BitrateStatistics& stats) { | 
| + statistics_ = stats; | 
| + aggregator_.OnStatsUpdated(); | 
| +} | 
| + | 
| +const BitrateStatistics& | 
| +RTPSender::BitrateAggregator::BitrateObserver::statistics() const { | 
| + return statistics_; | 
| +} | 
| RTPSender::RTPSender( | 
| bool audio, | 
| @@ -132,8 +122,8 @@ RTPSender::RTPSender( | 
| clock_delta_ms_(clock_->TimeInMilliseconds() - | 
| TickTime::MillisecondTimestamp()), | 
| random_(clock_->TimeInMicroseconds()), | 
| - bitrates_(new BitrateAggregator(bitrate_callback)), | 
| - total_bitrate_sent_(clock, bitrates_->total_bitrate_observer()), | 
| + bitrates_(bitrate_callback), | 
| + total_bitrate_sent_(clock, bitrates_.total_bitrate_observer()), | 
| audio_configured_(audio), | 
| audio_(audio ? new RTPSenderAudio(clock, this, audio_feedback) : nullptr), | 
| video_(audio ? nullptr : new RTPSenderVideo(clock, this)), | 
| @@ -141,7 +131,6 @@ RTPSender::RTPSender( | 
| transport_sequence_number_allocator_(sequence_number_allocator), | 
| transport_feedback_observer_(transport_feedback_observer), | 
| last_capture_time_ms_sent_(0), | 
| - send_critsect_(CriticalSectionWrapper::CreateCriticalSection()), | 
| transport_(transport), | 
| sending_media_(true), // Default to sending media. | 
| max_payload_length_(IP_PACKET_SIZE - 28), // Default is IP-v4/UDP. | 
| @@ -157,7 +146,7 @@ RTPSender::RTPSender( | 
| // NACK. | 
| nack_byte_count_times_(), | 
| nack_byte_count_(), | 
| - nack_bitrate_(clock, bitrates_->retransmit_bitrate_observer()), | 
| + nack_bitrate_(clock, bitrates_.retransmit_bitrate_observer()), | 
| packet_history_(clock), | 
| // Statistics | 
| statistics_crit_(CriticalSectionWrapper::CreateCriticalSection()), | 
| @@ -168,7 +157,7 @@ RTPSender::RTPSender( | 
| // RTP variables | 
| start_timestamp_forced_(false), | 
| start_timestamp_(0), | 
| - ssrc_db_(*SSRCDatabase::GetSSRCDatabase()), | 
| + ssrc_db_(SSRCDatabase::GetSSRCDatabase()), | 
| remote_ssrc_(0), | 
| sequence_number_forced_(false), | 
| ssrc_forced_(false), | 
| @@ -184,21 +173,35 @@ RTPSender::RTPSender( | 
| target_bitrate_(0) { | 
| memset(nack_byte_count_times_, 0, sizeof(nack_byte_count_times_)); | 
| memset(nack_byte_count_, 0, sizeof(nack_byte_count_)); | 
| - // We need to seed the random generator. | 
| + // We need to seed the random generator for BuildPaddingPacket() below. | 
| + // TODO(holmer,tommi): Note that TimeInMilliseconds might return 0 on Mac | 
| + // early on in the process. | 
| srand(static_cast<uint32_t>(clock_->TimeInMilliseconds())); | 
| - ssrc_ = ssrc_db_.CreateSSRC(); // Can't be 0. | 
| - ssrc_rtx_ = ssrc_db_.CreateSSRC(); // Can't be 0. | 
| - bitrates_->set_ssrc(ssrc_); | 
| + ssrc_ = ssrc_db_->CreateSSRC(); | 
| + RTC_DCHECK(ssrc_); | 
| + ssrc_rtx_ = ssrc_db_->CreateSSRC(); | 
| + RTC_DCHECK(ssrc_rtx_); | 
| + | 
| + bitrates_.set_ssrc(ssrc_); | 
| // Random start, 16 bits. Can't be 0. | 
| sequence_number_rtx_ = random_.Rand(1, kMaxInitRtpSeqNumber); | 
| sequence_number_ = random_.Rand(1, kMaxInitRtpSeqNumber); | 
| } | 
| RTPSender::~RTPSender() { | 
| + // TODO(tommi): Use a thread checker to ensure the object is created and | 
| + // deleted on the same thread. At the moment this isn't possible due to | 
| + // voe::ChannelOwner in voice engine. To reproduce, run: | 
| + // voe_auto_test --automated --gtest_filter=*MixManyChannelsForStressOpus | 
| + | 
| + // TODO(tommi,holmer): We don't grab locks in the dtor before accessing member | 
| + // variables but we grab them in all other methods. (what's the design?) | 
| + // Start documenting what thread we're on in what method so that it's easier | 
| + // to understand performance attributes and possibly remove locks. | 
| if (remote_ssrc_ != 0) { | 
| - ssrc_db_.ReturnSSRC(remote_ssrc_); | 
| + ssrc_db_->ReturnSSRC(remote_ssrc_); | 
| } | 
| - ssrc_db_.ReturnSSRC(ssrc_); | 
| + ssrc_db_->ReturnSSRC(ssrc_); | 
| SSRCDatabase::ReturnSSRCDatabase(); | 
| while (!payload_type_map_.empty()) { | 
| @@ -246,7 +249,7 @@ int32_t RTPSender::SetTransmissionTimeOffset(int32_t transmission_time_offset) { | 
| transmission_time_offset < -(0x800000 - 1)) { // Word24. | 
| return -1; | 
| } | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| transmission_time_offset_ = transmission_time_offset; | 
| return 0; | 
| } | 
| @@ -255,25 +258,25 @@ int32_t RTPSender::SetAbsoluteSendTime(uint32_t absolute_send_time) { | 
| if (absolute_send_time > 0xffffff) { // UWord24. | 
| return -1; | 
| } | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| absolute_send_time_ = absolute_send_time; | 
| return 0; | 
| } | 
| void RTPSender::SetVideoRotation(VideoRotation rotation) { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| rotation_ = rotation; | 
| } | 
| int32_t RTPSender::SetTransportSequenceNumber(uint16_t sequence_number) { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| transport_sequence_number_ = sequence_number; | 
| return 0; | 
| } | 
| int32_t RTPSender::RegisterRtpHeaderExtension(RTPExtensionType type, | 
| uint8_t id) { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| if (type == kRtpExtensionVideoRotation) { | 
| cvo_mode_ = kCVOInactive; | 
| return rtp_header_extension_map_.RegisterInactive(type, id); | 
| @@ -282,17 +285,17 @@ int32_t RTPSender::RegisterRtpHeaderExtension(RTPExtensionType type, | 
| } | 
| bool RTPSender::IsRtpHeaderExtensionRegistered(RTPExtensionType type) { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| return rtp_header_extension_map_.IsRegistered(type); | 
| } | 
| int32_t RTPSender::DeregisterRtpHeaderExtension(RTPExtensionType type) { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| return rtp_header_extension_map_.Deregister(type); | 
| } | 
| size_t RTPSender::RtpHeaderExtensionTotalLength() const { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| return rtp_header_extension_map_.GetTotalLengthInBytes(); | 
| } | 
| @@ -303,7 +306,7 @@ int32_t RTPSender::RegisterPayload( | 
| size_t channels, | 
| uint32_t rate) { | 
| assert(payload_name); | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| std::map<int8_t, RtpUtility::Payload*>::iterator it = | 
| payload_type_map_.find(payload_number); | 
| @@ -346,7 +349,7 @@ int32_t RTPSender::RegisterPayload( | 
| } | 
| int32_t RTPSender::DeRegisterSendPayload(int8_t payload_type) { | 
| - CriticalSectionScoped lock(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| std::map<int8_t, RtpUtility::Payload*>::iterator it = | 
| payload_type_map_.find(payload_type); | 
| @@ -361,12 +364,12 @@ int32_t RTPSender::DeRegisterSendPayload(int8_t payload_type) { | 
| } | 
| void RTPSender::SetSendPayloadType(int8_t payload_type) { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| payload_type_ = payload_type; | 
| } | 
| int8_t RTPSender::SendPayloadType() const { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| return payload_type_; | 
| } | 
| @@ -379,7 +382,7 @@ int32_t RTPSender::SetMaxPayloadLength(size_t max_payload_length, | 
| // Sanity check. | 
| RTC_DCHECK(max_payload_length >= 100 && max_payload_length <= IP_PACKET_SIZE) | 
| << "Invalid max payload length: " << max_payload_length; | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| max_payload_length_ = max_payload_length; | 
| packet_over_head_ = packet_over_head; | 
| return 0; | 
| @@ -388,7 +391,7 @@ int32_t RTPSender::SetMaxPayloadLength(size_t max_payload_length, | 
| size_t RTPSender::MaxDataPayloadLength() const { | 
| int rtx; | 
| { | 
| - CriticalSectionScoped rtx_lock(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| rtx = rtx_; | 
| } | 
| if (audio_configured_) { | 
| @@ -407,28 +410,28 @@ size_t RTPSender::MaxPayloadLength() const { | 
| uint16_t RTPSender::PacketOverHead() const { return packet_over_head_; } | 
| void RTPSender::SetRtxStatus(int mode) { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| rtx_ = mode; | 
| } | 
| int RTPSender::RtxStatus() const { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| return rtx_; | 
| } | 
| void RTPSender::SetRtxSsrc(uint32_t ssrc) { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| ssrc_rtx_ = ssrc; | 
| } | 
| uint32_t RTPSender::RtxSsrc() const { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| return ssrc_rtx_; | 
| } | 
| void RTPSender::SetRtxPayloadType(int payload_type, | 
| int associated_payload_type) { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| RTC_DCHECK_LE(payload_type, 127); | 
| RTC_DCHECK_LE(associated_payload_type, 127); | 
| if (payload_type < 0) { | 
| @@ -441,7 +444,7 @@ void RTPSender::SetRtxPayloadType(int payload_type, | 
| } | 
| std::pair<int, int> RTPSender::RtxPayloadType() const { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| for (const auto& kv : rtx_payload_type_map_) { | 
| if (kv.second == rtx_payload_type_) { | 
| return std::make_pair(rtx_payload_type_, kv.first); | 
| @@ -452,7 +455,7 @@ std::pair<int, int> RTPSender::RtxPayloadType() const { | 
| int32_t RTPSender::CheckPayloadType(int8_t payload_type, | 
| RtpVideoCodecTypes* video_type) { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| if (payload_type < 0) { | 
| LOG(LS_ERROR) << "Invalid payload_type " << payload_type; | 
| @@ -494,7 +497,7 @@ int32_t RTPSender::CheckPayloadType(int8_t payload_type, | 
| RTPSenderInterface::CVOMode RTPSender::ActivateCVORtpHeaderExtension() { | 
| if (cvo_mode_ == kCVOInactive) { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| if (rtp_header_extension_map_.SetActive(kRtpExtensionVideoRotation, true)) { | 
| cvo_mode_ = kCVOActivated; | 
| } | 
| @@ -513,7 +516,7 @@ int32_t RTPSender::SendOutgoingData(FrameType frame_type, | 
| uint32_t ssrc; | 
| { | 
| // Drop this packet if we're not sending media packets. | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| ssrc = ssrc_; | 
| if (!sending_media_) { | 
| return 0; | 
| @@ -565,7 +568,7 @@ int32_t RTPSender::SendOutgoingData(FrameType frame_type, | 
| size_t RTPSender::TrySendRedundantPayloads(size_t bytes_to_send) { | 
| { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| if ((rtx_ & kRtxRedundantPayloads) == 0) | 
| return 0; | 
| } | 
| @@ -627,7 +630,7 @@ size_t RTPSender::SendPadData(size_t bytes, | 
| int payload_type; | 
| bool over_rtx; | 
| { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| if (!timestamp_provided) { | 
| timestamp = timestamp_; | 
| capture_time_ms = capture_time_ms_; | 
| @@ -741,7 +744,7 @@ int32_t RTPSender::ReSendPacket(uint16_t packet_id, int64_t min_resend_time) { | 
| } | 
| int rtx = kRtxOff; | 
| { | 
| - CriticalSectionScoped lock(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| rtx = rtx_; | 
| } | 
| if (!PrepareAndSendPacket(data_buffer, length, capture_time_ms, | 
| @@ -839,7 +842,7 @@ bool RTPSender::ProcessNACKBitRate(uint32_t now) { | 
| const uint32_t kAvgIntervalMs = 1000; | 
| uint32_t target_bitrate = GetTargetBitrate(); | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| if (target_bitrate == 0) { | 
| return true; | 
| @@ -864,7 +867,7 @@ bool RTPSender::ProcessNACKBitRate(uint32_t now) { | 
| } | 
| void RTPSender::UpdateNACKBitRate(uint32_t bytes, int64_t now) { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| if (bytes == 0) | 
| return; | 
| nack_bitrate_.Update(bytes); | 
| @@ -900,7 +903,7 @@ bool RTPSender::TimeToSendPacket(uint16_t sequence_number, | 
| } | 
| int rtx; | 
| { | 
| - CriticalSectionScoped lock(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| rtx = rtx_; | 
| } | 
| return PrepareAndSendPacket(data_buffer, | 
| @@ -958,7 +961,7 @@ bool RTPSender::PrepareAndSendPacket(uint8_t* buffer, | 
| bool ret = SendPacketToNetwork(buffer_to_send_ptr, length, options); | 
| if (ret) { | 
| - CriticalSectionScoped lock(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| media_has_been_sent_ = true; | 
| } | 
| UpdateRtpStats(buffer_to_send_ptr, length, rtp_header, send_over_rtx, | 
| @@ -1018,7 +1021,7 @@ size_t RTPSender::TimeToSendPadding(size_t bytes) { | 
| if (audio_configured_ || bytes == 0) | 
| return 0; | 
| { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| if (!sending_media_) | 
| return 0; | 
| } | 
| @@ -1090,7 +1093,7 @@ int32_t RTPSender::SendToNetwork(uint8_t* buffer, | 
| return -1; | 
| { | 
| - CriticalSectionScoped lock(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| media_has_been_sent_ = true; | 
| } | 
| UpdateRtpStats(buffer, length, rtp_header, false, false); | 
| @@ -1105,7 +1108,7 @@ void RTPSender::UpdateDelayStatistics(int64_t capture_time_ms, int64_t now_ms) { | 
| int avg_delay_ms = 0; | 
| int max_delay_ms = 0; | 
| { | 
| - CriticalSectionScoped lock(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| ssrc = ssrc_; | 
| } | 
| { | 
| @@ -1131,7 +1134,7 @@ void RTPSender::UpdateDelayStatistics(int64_t capture_time_ms, int64_t now_ms) { | 
| } | 
| void RTPSender::ProcessBitrate() { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| total_bitrate_sent_.Process(); | 
| nack_bitrate_.Process(); | 
| if (audio_configured_) { | 
| @@ -1141,7 +1144,7 @@ void RTPSender::ProcessBitrate() { | 
| } | 
| size_t RTPSender::RTPHeaderLength() const { | 
| - CriticalSectionScoped lock(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| size_t rtp_header_length = kRtpHeaderLength; | 
| rtp_header_length += sizeof(uint32_t) * csrcs_.size(); | 
| rtp_header_length += RtpHeaderExtensionTotalLength(); | 
| @@ -1149,7 +1152,7 @@ size_t RTPSender::RTPHeaderLength() const { | 
| } | 
| uint16_t RTPSender::AllocateSequenceNumber(uint16_t packets_to_send) { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| uint16_t first_allocated_sequence_number = sequence_number_; | 
| sequence_number_ += packets_to_send; | 
| return first_allocated_sequence_number; | 
| @@ -1208,7 +1211,7 @@ int32_t RTPSender::BuildRTPheader(uint8_t* data_buffer, | 
| bool timestamp_provided, | 
| bool inc_sequence_number) { | 
| assert(payload_type >= 0); | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| if (timestamp_provided) { | 
| timestamp_ = start_timestamp_ + capture_timestamp; | 
| @@ -1515,7 +1518,7 @@ void RTPSender::UpdateTransmissionTimeOffset(uint8_t* rtp_packet, | 
| const RTPHeader& rtp_header, | 
| int64_t time_diff_ms) const { | 
| size_t offset; | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| switch (VerifyExtension(kRtpExtensionTransmissionTimeOffset, rtp_packet, | 
| rtp_packet_length, rtp_header, | 
| kTransmissionTimeOffsetLength, &offset)) { | 
| @@ -1541,7 +1544,7 @@ bool RTPSender::UpdateAudioLevel(uint8_t* rtp_packet, | 
| bool is_voiced, | 
| uint8_t dBov) const { | 
| size_t offset; | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| switch (VerifyExtension(kRtpExtensionAudioLevel, rtp_packet, | 
| rtp_packet_length, rtp_header, kAudioLevelLength, | 
| @@ -1566,7 +1569,7 @@ bool RTPSender::UpdateVideoRotation(uint8_t* rtp_packet, | 
| const RTPHeader& rtp_header, | 
| VideoRotation rotation) const { | 
| size_t offset; | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| switch (VerifyExtension(kRtpExtensionVideoRotation, rtp_packet, | 
| rtp_packet_length, rtp_header, kVideoRotationLength, | 
| @@ -1591,7 +1594,7 @@ void RTPSender::UpdateAbsoluteSendTime(uint8_t* rtp_packet, | 
| const RTPHeader& rtp_header, | 
| int64_t now_ms) const { | 
| size_t offset; | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| switch (VerifyExtension(kRtpExtensionAbsoluteSendTime, rtp_packet, | 
| rtp_packet_length, rtp_header, | 
| @@ -1618,7 +1621,7 @@ uint16_t RTPSender::UpdateTransportSequenceNumber( | 
| size_t rtp_packet_length, | 
| const RTPHeader& rtp_header) const { | 
| size_t offset; | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| switch (VerifyExtension(kRtpExtensionTransportSequenceNumber, rtp_packet, | 
| rtp_packet_length, rtp_header, | 
| @@ -1647,12 +1650,13 @@ void RTPSender::SetSendingStatus(bool enabled) { | 
| // Will be ignored if it's already configured via API. | 
| SetStartTimestamp(RTPtime, false); | 
| } else { | 
| - CriticalSectionScoped lock(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| if (!ssrc_forced_) { | 
| // Generate a new SSRC. | 
| - ssrc_db_.ReturnSSRC(ssrc_); | 
| - ssrc_ = ssrc_db_.CreateSSRC(); // Can't be 0. | 
| - bitrates_->set_ssrc(ssrc_); | 
| + ssrc_db_->ReturnSSRC(ssrc_); | 
| + ssrc_ = ssrc_db_->CreateSSRC(); | 
| + RTC_DCHECK(ssrc_); | 
| 
stefan-webrtc
2016/01/26 13:19:52
I prefer > 0 or != 0.
 
tommi
2016/01/30 10:53:41
Done.
 | 
| + bitrates_.set_ssrc(ssrc_); | 
| } | 
| // Don't initialize seq number if SSRC passed externally. | 
| if (!sequence_number_forced_ && !ssrc_forced_) { | 
| @@ -1663,22 +1667,22 @@ void RTPSender::SetSendingStatus(bool enabled) { | 
| } | 
| void RTPSender::SetSendingMediaStatus(bool enabled) { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| sending_media_ = enabled; | 
| } | 
| bool RTPSender::SendingMedia() const { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| return sending_media_; | 
| } | 
| uint32_t RTPSender::Timestamp() const { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| return timestamp_; | 
| } | 
| void RTPSender::SetStartTimestamp(uint32_t timestamp, bool force) { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| if (force) { | 
| start_timestamp_forced_ = true; | 
| start_timestamp_ = timestamp; | 
| @@ -1690,58 +1694,59 @@ void RTPSender::SetStartTimestamp(uint32_t timestamp, bool force) { | 
| } | 
| uint32_t RTPSender::StartTimestamp() const { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| return start_timestamp_; | 
| } | 
| uint32_t RTPSender::GenerateNewSSRC() { | 
| // If configured via API, return 0. | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| if (ssrc_forced_) { | 
| return 0; | 
| } | 
| - ssrc_ = ssrc_db_.CreateSSRC(); // Can't be 0. | 
| - bitrates_->set_ssrc(ssrc_); | 
| + ssrc_ = ssrc_db_->CreateSSRC(); | 
| + RTC_DCHECK(ssrc_); | 
| 
stefan-webrtc
2016/01/26 13:19:52
same here
 
tommi
2016/01/30 10:53:41
Done.
 | 
| + bitrates_.set_ssrc(ssrc_); | 
| return ssrc_; | 
| } | 
| void RTPSender::SetSSRC(uint32_t ssrc) { | 
| // This is configured via the API. | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| if (ssrc_ == ssrc && ssrc_forced_) { | 
| return; // Since it's same ssrc, don't reset anything. | 
| } | 
| ssrc_forced_ = true; | 
| - ssrc_db_.ReturnSSRC(ssrc_); | 
| - ssrc_db_.RegisterSSRC(ssrc); | 
| + ssrc_db_->ReturnSSRC(ssrc_); | 
| + ssrc_db_->RegisterSSRC(ssrc); | 
| ssrc_ = ssrc; | 
| - bitrates_->set_ssrc(ssrc_); | 
| + bitrates_.set_ssrc(ssrc_); | 
| if (!sequence_number_forced_) { | 
| sequence_number_ = random_.Rand(1, kMaxInitRtpSeqNumber); | 
| } | 
| } | 
| uint32_t RTPSender::SSRC() const { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| return ssrc_; | 
| } | 
| void RTPSender::SetCsrcs(const std::vector<uint32_t>& csrcs) { | 
| assert(csrcs.size() <= kRtpCsrcSize); | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| csrcs_ = csrcs; | 
| } | 
| void RTPSender::SetSequenceNumber(uint16_t seq) { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| sequence_number_forced_ = true; | 
| sequence_number_ = seq; | 
| } | 
| uint16_t RTPSender::SequenceNumber() const { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| return sequence_number_; | 
| } | 
| @@ -1818,7 +1823,7 @@ int32_t RTPSender::SetFecParameters( | 
| void RTPSender::BuildRtxPacket(uint8_t* buffer, size_t* length, | 
| uint8_t* buffer_rtx) { | 
| - CriticalSectionScoped cs(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| uint8_t* data_buffer_rtx = buffer_rtx; | 
| // Add RTX header. | 
| RtpUtility::RtpHeaderParser rtp_parser( | 
| @@ -1873,7 +1878,7 @@ uint32_t RTPSender::BitrateSent() const { | 
| void RTPSender::SetRtpState(const RtpState& rtp_state) { | 
| SetStartTimestamp(rtp_state.start_timestamp, true); | 
| - CriticalSectionScoped lock(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| sequence_number_ = rtp_state.sequence_number; | 
| sequence_number_forced_ = true; | 
| timestamp_ = rtp_state.timestamp; | 
| @@ -1883,7 +1888,7 @@ void RTPSender::SetRtpState(const RtpState& rtp_state) { | 
| } | 
| RtpState RTPSender::GetRtpState() const { | 
| - CriticalSectionScoped lock(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| RtpState state; | 
| state.sequence_number = sequence_number_; | 
| @@ -1897,12 +1902,12 @@ RtpState RTPSender::GetRtpState() const { | 
| } | 
| void RTPSender::SetRtxRtpState(const RtpState& rtp_state) { | 
| - CriticalSectionScoped lock(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| sequence_number_rtx_ = rtp_state.sequence_number; | 
| } | 
| RtpState RTPSender::GetRtxRtpState() const { | 
| - CriticalSectionScoped lock(send_critsect_.get()); | 
| + rtc::CritScope lock(&send_critsect_); | 
| RtpState state; | 
| state.sequence_number = sequence_number_rtx_; |