Add thread checks to ReceiveStatisticsProxy that reflect

design comments.

Change-Id: I4dd2b0be006440500cc7744de0e952263531ccd2
Bug: webrtc:8929
Reviewed-on: https://webrtc-review.googlesource.com/54940
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22179}

video/receive_statistics_proxy.cc

@@ -113,6 +113,8 @@ ReceiveStatisticsProxy::ReceiveStatisticsProxy(
       avg_rtt_ms_(0),
       last_content_type_(VideoContentType::UNSPECIFIED),
       timing_frame_info_counter_(kMovingMaxWindowMs) {
+  decode_thread_.DetachFromThread();
+  network_thread_.DetachFromThread();
   stats_.ssrc = config_.rtp.remote_ssrc;
   // TODO(brandtr): Replace |rtx_stats_| with a single instance of
   // StreamDataCounters.
@@ -122,10 +124,19 @@ ReceiveStatisticsProxy::ReceiveStatisticsProxy(
 }
 
 ReceiveStatisticsProxy::~ReceiveStatisticsProxy() {
+  RTC_DCHECK_RUN_ON(&main_thread_);
+  // In case you're reading this wondering "hmm... we're on the main thread but
+  // calling a method that needs to be called on the decoder thread...", then
+  // here's what's going on:
+  // - The decoder thread has been stopped and DecoderThreadStopped() has been
+  //   called.
+  // - The decode_thread_ thread checker has been detached, and will now become
+  //   attached to the current thread, which is OK since we're in the dtor.
   UpdateHistograms();
 }
 
 void ReceiveStatisticsProxy::UpdateHistograms() {
+  RTC_DCHECK_RUN_ON(&decode_thread_);
   std::ostringstream logStream;
   int stream_duration_sec = (clock_->TimeInMilliseconds() - start_ms_) / 1000;
   if (stats_.frame_counts.key_frames > 0 ||
@@ -450,6 +461,8 @@ void ReceiveStatisticsProxy::UpdateHistograms() {
 }
 
 void ReceiveStatisticsProxy::QualitySample() {
+  RTC_DCHECK_RUN_ON(&network_thread_);
+
   int64_t now = clock_->TimeInMilliseconds();
   if (last_sample_time_ + kMinSampleLengthMs > now)
     return;
@@ -559,6 +572,7 @@ void ReceiveStatisticsProxy::OnDecoderImplementationName(
 }
 void ReceiveStatisticsProxy::OnIncomingRate(unsigned int framerate,
                                             unsigned int bitrate_bps) {
+  RTC_DCHECK_RUN_ON(&network_thread_);
   rtc::CritScope lock(&crit_);
   if (stats_.rtp_stats.first_packet_time_ms != -1)
     QualitySample();
@@ -783,6 +797,7 @@ void ReceiveStatisticsProxy::OnDiscardedPacketsUpdated(int discarded_packets) {
 void ReceiveStatisticsProxy::OnPreDecode(
     const EncodedImage& encoded_image,
     const CodecSpecificInfo* codec_specific_info) {
+  RTC_DCHECK_RUN_ON(&decode_thread_);
   if (!codec_specific_info || encoded_image.qp_ == -1) {
     return;
   }
@@ -839,6 +854,15 @@ void ReceiveStatisticsProxy::OnRttUpdate(int64_t avg_rtt_ms,
   avg_rtt_ms_ = avg_rtt_ms;
 }
 
+void ReceiveStatisticsProxy::DecoderThreadStarting() {
+  RTC_DCHECK_RUN_ON(&main_thread_);
+}
+
+void ReceiveStatisticsProxy::DecoderThreadStopped() {
+  RTC_DCHECK_RUN_ON(&main_thread_);
+  decode_thread_.DetachFromThread();
+}
+
 ReceiveStatisticsProxy::ContentSpecificStats::ContentSpecificStats()
     : interframe_delay_percentiles(kMaxCommonInterframeDelayMs) {}
 

video/receive_statistics_proxy.h

@@ -26,6 +26,7 @@
 #include "rtc_base/rate_statistics.h"
 #include "rtc_base/ratetracker.h"
 #include "rtc_base/thread_annotations.h"
+#include "rtc_base/thread_checker.h"
 #include "video/quality_threshold.h"
 #include "video/report_block_stats.h"
 #include "video/stats_counter.h"
@@ -98,6 +99,11 @@ class ReceiveStatisticsProxy : public VCMReceiveStatisticsCallback,
   // Implements CallStatsObserver.
   void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override;
 
+  // Notification methods that are used to check our internal state and validate
+  // threading assumptions. These are called by VideoReceiveStream.
+  void DecoderThreadStarting();
+  void DecoderThreadStopped();
+
  private:
   struct SampleCounter {
     SampleCounter() : sum(0), num_samples(0) {}
@@ -179,7 +185,7 @@ class ReceiveStatisticsProxy : public VCMReceiveStatisticsCallback,
   MaxCounter freq_offset_counter_ RTC_GUARDED_BY(crit_);
   int64_t first_report_block_time_ms_ RTC_GUARDED_BY(crit_);
   ReportBlockStats report_block_stats_ RTC_GUARDED_BY(crit_);
-  QpCounters qp_counters_;  // Only accessed on the decoding thread.
+  QpCounters qp_counters_ RTC_GUARDED_BY(decode_thread_);
   std::map<uint32_t, StreamDataCounters> rtx_stats_ RTC_GUARDED_BY(crit_);
   int64_t avg_rtt_ms_ RTC_GUARDED_BY(crit_);
   mutable std::map<int64_t, size_t> frame_window_ RTC_GUARDED_BY(&crit_);
@@ -191,6 +197,9 @@ class ReceiveStatisticsProxy : public VCMReceiveStatisticsCallback,
   mutable rtc::MovingMaxCounter<TimingFrameInfo> timing_frame_info_counter_
       RTC_GUARDED_BY(&crit_);
   rtc::Optional<int> num_unique_frames_ RTC_GUARDED_BY(crit_);
+  rtc::ThreadChecker decode_thread_;
+  rtc::ThreadChecker network_thread_;
+  rtc::ThreadChecker main_thread_;
 };
 
 }  // namespace webrtc

video/video_receive_stream.cc

@@ -227,6 +227,7 @@ void VideoReceiveStream::Start() {
 
   // Start the decode thread
   video_receiver_.DecoderThreadStarting();
+  stats_proxy_.DecoderThreadStarting();
   decode_thread_.Start();
   rtp_video_stream_receiver_.StartReceive();
 }
@@ -251,6 +252,7 @@ void VideoReceiveStream::Stop() {
 
     decode_thread_.Stop();
     video_receiver_.DecoderThreadStopped();
+    stats_proxy_.DecoderThreadStopped();
     // Deregister external decoders so they are no longer running during
     // destruction. This effectively stops the VCM since the decoder thread is
     // stopped, the VCM is deregistered and no asynchronous decoder threads are