diff --git a/webrtc/base/random.h b/webrtc/base/random.h index 647b84c9c9..cb7b9ebe4a 100644 --- a/webrtc/base/random.h +++ b/webrtc/base/random.h @@ -21,6 +21,17 @@ namespace webrtc { class Random { public: + // TODO(tommi): Change this so that the seed can be initialized internally, + // e.g. by offering two ways of constructing or offer a static method that + // returns a seed that's suitable for initialization. + // The problem now is that callers are calling clock_->TimeInMicroseconds() + // which calls TickTime::Now().Ticks(), which can return a very low value on + // Mac and can result in a seed of 0 after conversion to microseconds. + // Besides the quality of the random seed being poor, this also requires + // the client to take on extra dependencies to generate a seed. + // If we go for a static seed generator in Random, we can use something from + // webrtc/base and make sure that it works the same way across platforms. + // See also discussion here: https://codereview.webrtc.org/1623543002/ explicit Random(uint64_t seed); // Return pseudo-random integer of the specified type. diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index ba5b98148d..6f79235b6e 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) {} +RTPSender::BitrateAggregator::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_); +void RTPSender::BitrateAggregator::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_; - } +Bitrate::Observer* RTPSender::BitrateAggregator::total_bitrate_observer() { + return &total_bitrate_observer_; +} +Bitrate::Observer* RTPSender::BitrateAggregator::retransmit_bitrate_observer() { + return &retransmit_bitrate_observer_; +} - void set_ssrc(uint32_t ssrc) { ssrc_ = ssrc; } +void RTPSender::BitrateAggregator::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) {} +RTPSender::BitrateAggregator::BitrateObserver::BitrateObserver( + const BitrateAggregator& aggregator) + : aggregator_(aggregator) {} - // Implements Bitrate::Observer. - void BitrateUpdated(const BitrateStatistics& stats) override { - statistics_ = stats; - aggregator_.OnStatsUpdated(); - } +// Implements Bitrate::Observer. +void RTPSender::BitrateAggregator::BitrateObserver::BitrateUpdated( + const BitrateStatistics& stats) { + statistics_ = stats; + aggregator_.OnStatsUpdated(); +} - BitrateStatistics statistics() const { return statistics_; } - - private: - BitrateStatistics statistics_; - const BitrateAggregator& aggregator_; - }; - - BitrateStatisticsObserver* const callback_; - BitrateObserver total_bitrate_observer_; - BitrateObserver retransmit_bitrate_observer_; - uint32_t ssrc_; -}; +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(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_ != 0); + ssrc_rtx_ = ssrc_db_->CreateSSRC(); + RTC_DCHECK(ssrc_rtx_ != 0); + + 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::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::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 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 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_; @@ -745,7 +748,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, @@ -843,7 +846,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; @@ -868,7 +871,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); @@ -904,7 +907,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, @@ -962,7 +965,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, @@ -1022,7 +1025,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; } @@ -1108,7 +1111,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); @@ -1123,7 +1126,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_; } { @@ -1149,7 +1152,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_) { @@ -1159,7 +1162,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(); @@ -1167,7 +1170,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; @@ -1226,7 +1229,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; @@ -1533,7 +1536,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)) { @@ -1559,7 +1562,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, @@ -1584,7 +1587,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, @@ -1609,7 +1612,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, @@ -1636,7 +1639,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, @@ -1665,12 +1668,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_ != 0); + bitrates_.set_ssrc(ssrc_); } // Don't initialize seq number if SSRC passed externally. if (!sequence_number_forced_ && !ssrc_forced_) { @@ -1681,22 +1685,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; @@ -1708,58 +1712,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_ != 0); + 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& 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_; } @@ -1836,7 +1841,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( @@ -1890,7 +1895,7 @@ uint32_t RTPSender::BitrateSent() const { } void RTPSender::SetRtpState(const RtpState& rtp_state) { - CriticalSectionScoped lock(send_critsect_.get()); + rtc::CritScope lock(&send_critsect_); sequence_number_ = rtp_state.sequence_number; sequence_number_forced_ = true; timestamp_ = rtp_state.timestamp; @@ -1900,7 +1905,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_; @@ -1914,12 +1919,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_; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.h b/webrtc/modules/rtp_rtcp/source/rtp_sender.h index a672a06398..49d98729ee 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.h @@ -16,6 +16,7 @@ #include #include +#include "webrtc/base/criticalsection.h" #include "webrtc/base/random.h" #include "webrtc/base/thread_annotations.h" #include "webrtc/common_types.h" @@ -30,8 +31,6 @@ namespace webrtc { -class BitrateAggregator; -class CriticalSectionWrapper; class RTPSenderAudio; class RTPSenderVideo; class RtcEventLog; @@ -196,7 +195,7 @@ class RTPSender : public RTPSenderInterface { const RTPHeader& rtp_header, size_t extension_length_bytes, size_t* extension_offset) const - EXCLUSIVE_LOCKS_REQUIRED(send_critsect_.get()); + EXCLUSIVE_LOCKS_REQUIRED(send_critsect_); bool UpdateAudioLevel(uint8_t* rtp_packet, size_t rtp_packet_length, @@ -386,22 +385,54 @@ class RTPSender : public RTPSenderInterface { bool is_retransmit); bool IsFecPacket(const uint8_t* buffer, const RTPHeader& header) const; - Clock* clock_; - int64_t clock_delta_ms_; + class BitrateAggregator { + public: + explicit BitrateAggregator(BitrateStatisticsObserver* bitrate_callback); + + void OnStatsUpdated() const; + + Bitrate::Observer* total_bitrate_observer(); + Bitrate::Observer* retransmit_bitrate_observer(); + void set_ssrc(uint32_t 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); + + // Implements Bitrate::Observer. + void BitrateUpdated(const BitrateStatistics& stats) override; + const BitrateStatistics& statistics() const; + + private: + BitrateStatistics statistics_; + const BitrateAggregator& aggregator_; + }; + + BitrateStatisticsObserver* const callback_; + BitrateObserver total_bitrate_observer_; + BitrateObserver retransmit_bitrate_observer_; + uint32_t ssrc_; + }; + + Clock* const clock_; + const int64_t clock_delta_ms_; Random random_ GUARDED_BY(send_critsect_); - rtc::scoped_ptr bitrates_; + BitrateAggregator bitrates_; Bitrate total_bitrate_sent_; const bool audio_configured_; - rtc::scoped_ptr audio_; - rtc::scoped_ptr video_; + const rtc::scoped_ptr audio_; + const rtc::scoped_ptr video_; RtpPacketSender* const paced_sender_; TransportSequenceNumberAllocator* const transport_sequence_number_allocator_; TransportFeedbackObserver* const transport_feedback_observer_; int64_t last_capture_time_ms_sent_; - rtc::scoped_ptr send_critsect_; + rtc::CriticalSection send_critsect_; Transport *transport_; bool sending_media_ GUARDED_BY(send_critsect_); @@ -440,7 +471,7 @@ class RTPSender : public RTPSenderInterface { // RTP variables bool start_timestamp_forced_ GUARDED_BY(send_critsect_); uint32_t start_timestamp_ GUARDED_BY(send_critsect_); - SSRCDatabase& ssrc_db_ GUARDED_BY(send_critsect_); + SSRCDatabase* const ssrc_db_; uint32_t remote_ssrc_ GUARDED_BY(send_critsect_); bool sequence_number_forced_ GUARDED_BY(send_critsect_); uint16_t sequence_number_ GUARDED_BY(send_critsect_); diff --git a/webrtc/modules/rtp_rtcp/source/ssrc_database.cc b/webrtc/modules/rtp_rtcp/source/ssrc_database.cc index fb02b7ef12..f1d1549e27 100644 --- a/webrtc/modules/rtp_rtcp/source/ssrc_database.cc +++ b/webrtc/modules/rtp_rtcp/source/ssrc_database.cc @@ -11,15 +11,9 @@ #include "webrtc/modules/rtp_rtcp/source/ssrc_database.h" #include "webrtc/base/checks.h" -#include "webrtc/system_wrappers/include/clock.h" -#include "webrtc/system_wrappers/include/critical_section_wrapper.h" +#include "webrtc/system_wrappers/include/tick_util.h" namespace webrtc { -namespace { -uint64_t Seed() { - return Clock::GetRealTimeClock()->TimeInMicroseconds(); -} -} // namespace SSRCDatabase* SSRCDatabase::GetSSRCDatabase() { return GetStaticInstance(kAddRef); @@ -30,7 +24,7 @@ void SSRCDatabase::ReturnSSRCDatabase() { } uint32_t SSRCDatabase::CreateSSRC() { - CriticalSectionScoped lock(crit_.get()); + rtc::CritScope lock(&crit_); while (true) { // Try until get a new ssrc. // 0 and 0xffffffff are invalid values for SSRC. @@ -42,19 +36,17 @@ uint32_t SSRCDatabase::CreateSSRC() { } void SSRCDatabase::RegisterSSRC(uint32_t ssrc) { - CriticalSectionScoped lock(crit_.get()); + rtc::CritScope lock(&crit_); ssrcs_.insert(ssrc); } void SSRCDatabase::ReturnSSRC(uint32_t ssrc) { - CriticalSectionScoped lock(crit_.get()); + rtc::CritScope lock(&crit_); ssrcs_.erase(ssrc); } -SSRCDatabase::SSRCDatabase() - : crit_(CriticalSectionWrapper::CreateCriticalSection()), random_(Seed()) {} +SSRCDatabase::SSRCDatabase() : random_(TickTime::Now().Ticks()) {} -SSRCDatabase::~SSRCDatabase() { -} +SSRCDatabase::~SSRCDatabase() {} } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/ssrc_database.h b/webrtc/modules/rtp_rtcp/source/ssrc_database.h index 7a3133638d..2f6357aec0 100644 --- a/webrtc/modules/rtp_rtcp/source/ssrc_database.h +++ b/webrtc/modules/rtp_rtcp/source/ssrc_database.h @@ -13,14 +13,21 @@ #include +#include "webrtc/base/criticalsection.h" #include "webrtc/base/random.h" -#include "webrtc/base/scoped_ptr.h" +#include "webrtc/base/thread_annotations.h" #include "webrtc/system_wrappers/include/static_instance.h" #include "webrtc/typedefs.h" namespace webrtc { -class CriticalSectionWrapper; +// TODO(tommi, holmer): Look into whether we can eliminate locking in this +// class or the class itself completely once voice engine doesn't rely on it. +// At the moment voe_auto_test requires locking, but it's not clear if that's +// an issue with the test code or if it reflects real world usage or if that's +// the best design performance wise. +// If we do decide to keep the class, we should at least get rid of using +// StaticInstance. class SSRCDatabase { public: static SSRCDatabase* GetSSRCDatabase(); @@ -32,19 +39,23 @@ class SSRCDatabase { protected: SSRCDatabase(); - virtual ~SSRCDatabase(); + ~SSRCDatabase(); static SSRCDatabase* CreateInstance() { return new SSRCDatabase(); } - private: // Friend function to allow the SSRC destructor to be accessed from the // template class. friend SSRCDatabase* GetStaticInstance( CountOperation count_operation); - rtc::scoped_ptr crit_; + private: + rtc::CriticalSection crit_; Random random_ GUARDED_BY(crit_); std::set ssrcs_ GUARDED_BY(crit_); + // 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 }; } // namespace webrtc