From b615d1af905f63e656994f3e895b3aa845b592d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Mon, 27 Aug 2018 12:32:21 +0200 Subject: [PATCH] Add lock annotations in StreamStatisticianImpl Also consolidate lock operations to public methods only, moving one CritScope out of UpdateCounters (private) up to IncomingPacket (public). Bug: webrtc:7135 Change-Id: I458857d3cfa49961fa34196ffe02cdefd83cec10 Reviewed-on: https://webrtc-review.googlesource.com/96122 Reviewed-by: Danil Chapovalov Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/master@{#24443} --- .../source/receive_statistics_impl.cc | 8 +++- .../rtp_rtcp/source/receive_statistics_impl.h | 42 ++++++++++--------- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/modules/rtp_rtcp/source/receive_statistics_impl.cc b/modules/rtp_rtcp/source/receive_statistics_impl.cc index 362a7cfb4d..dd3ee55088 100644 --- a/modules/rtp_rtcp/source/receive_statistics_impl.cc +++ b/modules/rtp_rtcp/source/receive_statistics_impl.cc @@ -57,7 +57,12 @@ StreamStatisticianImpl::~StreamStatisticianImpl() = default; void StreamStatisticianImpl::IncomingPacket(const RTPHeader& header, size_t packet_length, bool retransmitted) { - auto counters = UpdateCounters(header, packet_length, retransmitted); + StreamDataCounters counters; + { + rtc::CritScope cs(&stream_lock_); + + counters = UpdateCounters(header, packet_length, retransmitted); + } rtp_callback_->DataCountersUpdated(counters, ssrc_); } @@ -65,7 +70,6 @@ StreamDataCounters StreamStatisticianImpl::UpdateCounters( const RTPHeader& header, size_t packet_length, bool retransmitted) { - rtc::CritScope cs(&stream_lock_); bool in_order = InOrderPacketInternal(header.sequenceNumber); RTC_DCHECK_EQ(ssrc_, header.ssrc); incoming_bitrate_.Update(packet_length, clock_->TimeInMilliseconds()); diff --git a/modules/rtp_rtcp/source/receive_statistics_impl.h b/modules/rtp_rtcp/source/receive_statistics_impl.h index 5559b7c287..b0235ed88b 100644 --- a/modules/rtp_rtcp/source/receive_statistics_impl.h +++ b/modules/rtp_rtcp/source/receive_statistics_impl.h @@ -48,40 +48,44 @@ class StreamStatisticianImpl : public StreamStatistician { void SetMaxReorderingThreshold(int max_reordering_threshold); private: - bool InOrderPacketInternal(uint16_t sequence_number) const; + bool InOrderPacketInternal(uint16_t sequence_number) const + RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_); RtcpStatistics CalculateRtcpStatistics() RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_); - void UpdateJitter(const RTPHeader& header, NtpTime receive_time); + void UpdateJitter(const RTPHeader& header, NtpTime receive_time) + RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_); StreamDataCounters UpdateCounters(const RTPHeader& rtp_header, size_t packet_length, - bool retransmitted); + bool retransmitted) + RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_); const uint32_t ssrc_; Clock* const clock_; rtc::CriticalSection stream_lock_; - RateStatistics incoming_bitrate_; - int max_reordering_threshold_; // In number of packets or sequence numbers. + RateStatistics incoming_bitrate_ RTC_GUARDED_BY(&stream_lock_); + // In number of packets or sequence numbers. + int max_reordering_threshold_ RTC_GUARDED_BY(&stream_lock_); // Stats on received RTP packets. - uint32_t jitter_q4_; - uint32_t cumulative_loss_; + uint32_t jitter_q4_ RTC_GUARDED_BY(&stream_lock_); + uint32_t cumulative_loss_ RTC_GUARDED_BY(&stream_lock_); - int64_t last_receive_time_ms_; - NtpTime last_receive_time_ntp_; - uint32_t last_received_timestamp_; - uint16_t received_seq_first_; - uint16_t received_seq_max_; - uint16_t received_seq_wraps_; + int64_t last_receive_time_ms_ RTC_GUARDED_BY(&stream_lock_); + NtpTime last_receive_time_ntp_ RTC_GUARDED_BY(&stream_lock_); + uint32_t last_received_timestamp_ RTC_GUARDED_BY(&stream_lock_); + uint16_t received_seq_first_ RTC_GUARDED_BY(&stream_lock_); + uint16_t received_seq_max_ RTC_GUARDED_BY(&stream_lock_); + uint16_t received_seq_wraps_ RTC_GUARDED_BY(&stream_lock_); // Current counter values. - size_t received_packet_overhead_; - StreamDataCounters receive_counters_; + size_t received_packet_overhead_ RTC_GUARDED_BY(&stream_lock_); + StreamDataCounters receive_counters_ RTC_GUARDED_BY(&stream_lock_); // Counter values when we sent the last report. - uint32_t last_report_inorder_packets_; - uint32_t last_report_old_packets_; - uint16_t last_report_seq_max_; - RtcpStatistics last_reported_statistics_; + uint32_t last_report_inorder_packets_ RTC_GUARDED_BY(&stream_lock_); + uint32_t last_report_old_packets_ RTC_GUARDED_BY(&stream_lock_); + uint16_t last_report_seq_max_ RTC_GUARDED_BY(&stream_lock_); + RtcpStatistics last_reported_statistics_ RTC_GUARDED_BY(&stream_lock_); // stream_lock_ shouldn't be held when calling callbacks. RtcpStatisticsCallback* const rtcp_callback_;