Preparational work for an upcoming addition of a threadchecking scheme for APM

webrtc/modules/audio_processing/audio_processing_impl.cc

Index: webrtc/modules/audio_processing/audio_processing_impl.cc
diff --git a/webrtc/modules/audio_processing/audio_processing_impl.cc b/webrtc/modules/audio_processing/audio_processing_impl.cc
index b98aad23da37cd98b4df153234ea1f2980ef78d4..d6795a518705b54c6627caaeb4fe62e9ff2bbcec 100644
--- a/webrtc/modules/audio_processing/audio_processing_impl.cc
+++ b/webrtc/modules/audio_processing/audio_processing_impl.cc
@@ -222,13 +222,16 @@ AudioProcessingImpl::AudioProcessingImpl(const Config& config,
       beamformer_(beamformer),
       array_geometry_(config.Get<Beamforming>().array_geometry),
       intelligibility_enabled_(config.Get<Intelligibility>().enabled) {
-  echo_cancellation_ = new EchoCancellationImpl(this, crit_);
+  echo_cancellation_ =
+      new EchoCancellationImpl(this, crit_, &render_thread_, &capture_thread_);
   component_list_.push_back(echo_cancellation_);
 
-  echo_control_mobile_ = new EchoControlMobileImpl(this, crit_);
+  echo_control_mobile_ =
+      new EchoControlMobileImpl(this, crit_, &render_thread_, &capture_thread_);
   component_list_.push_back(echo_control_mobile_);
 
-  gain_control_ = new GainControlImpl(this, crit_);
+  gain_control_ =
+      new GainControlImpl(this, crit_, &render_thread_, &capture_thread_);
   component_list_.push_back(gain_control_);
 
   high_pass_filter_ = new HighPassFilterImpl(this, crit_);
@@ -237,15 +240,18 @@ AudioProcessingImpl::AudioProcessingImpl(const Config& config,
   level_estimator_ = new LevelEstimatorImpl(this, crit_);
   component_list_.push_back(level_estimator_);
 
-  noise_suppression_ = new NoiseSuppressionImpl(this, crit_);
+  noise_suppression_ = new NoiseSuppressionImpl(this, crit_, &capture_thread_);
   component_list_.push_back(noise_suppression_);
 
-  voice_detection_ = new VoiceDetectionImpl(this, crit_);
+  voice_detection_ = new VoiceDetectionImpl(this, crit_, &capture_thread_);
   component_list_.push_back(voice_detection_);
 
   gain_control_for_new_agc_.reset(new GainControlForNewAgc(gain_control_));
 
   SetExtraOptions(config);
+
+  render_thread_.DetachFromThread();
+  capture_thread_.DetachFromThread();

[kwiberg-webrtc, 2015/10/26 13:57:56]

Move these as high up as possible, so that you never expose the half-initialized
state to your underlings? That way, it'll fail if one of them calls
CalledOnValidThread in its constructor.

[peah-webrtc, 2015/11/05 11:47:33]

I'm not convinced this is the right thing to do. If the work in the construction
would only be to initialize the state, it may be fine. But if the constructors
do more than just that and also calls other methods that have threadcheckers in
them, these calls will reinitialize the thread checkers to again point to the
initialization threads, which will cause things to start failing once the
threads are called using the render and capture threads. Does that make sense?

However, since it seems to work, I moved them according to your recommendation.

Done.

[kwiberg-webrtc, 2015/11/08 09:50:51]

Yes, that's exactly the sort of behavior you protect against by moving it up.
Having it very early is a proof that none of the potential issues you mentioned
are in fact happening.

 }
 
 AudioProcessingImpl::~AudioProcessingImpl() {
@@ -452,7 +458,6 @@ int AudioProcessingImpl::InitializeLocked(const ProcessingConfig& config) {
   return InitializeLocked();
 }
 
-
 void AudioProcessingImpl::SetExtraOptions(const Config& config) {
   CriticalSectionScoped crit_scoped(crit_);
   for (auto item : component_list_) {
@@ -488,6 +493,7 @@ int AudioProcessingImpl::num_output_channels() const {
 
 void AudioProcessingImpl::set_output_will_be_muted(bool muted) {
   CriticalSectionScoped lock(crit_);
+  RTC_DCHECK(capture_thread_.CalledOnValidThread());
   output_will_be_muted_ = muted;
   if (agc_manager_.get()) {
     agc_manager_->SetCaptureMuted(output_will_be_muted_);
@@ -503,6 +509,7 @@ int AudioProcessingImpl::ProcessStream(const float* const* src,
                                        ChannelLayout output_layout,
                                        float* const* dest) {
   CriticalSectionScoped crit_scoped(crit_);
+  RTC_DCHECK(capture_thread_.CalledOnValidThread());
   StreamConfig input_stream = shared_state_.api_format_.input_stream();
   input_stream.set_sample_rate_hz(input_sample_rate_hz);
   input_stream.set_num_channels(ChannelsFromLayout(input_layout));
@@ -524,6 +531,7 @@ int AudioProcessingImpl::ProcessStream(const float* const* src,
                                        const StreamConfig& output_config,
                                        float* const* dest) {
   CriticalSectionScoped crit_scoped(crit_);
+  RTC_DCHECK(capture_thread_.CalledOnValidThread());
   if (!src || !dest) {
     return kNullPointerError;
   }
@@ -575,6 +583,7 @@ int AudioProcessingImpl::ProcessStream(const float* const* src,
 
 int AudioProcessingImpl::ProcessStream(AudioFrame* frame) {
   CriticalSectionScoped crit_scoped(crit_);
+  RTC_DCHECK(capture_thread_.CalledOnValidThread());
   echo_cancellation_->ReadQueuedRenderData();
   echo_control_mobile_->ReadQueuedRenderData();
   gain_control_->ReadQueuedRenderData();
@@ -719,6 +728,7 @@ int AudioProcessingImpl::AnalyzeReverseStream(const float* const* data,
                                               size_t samples_per_channel,
                                               int rev_sample_rate_hz,
                                               ChannelLayout layout) {
+  RTC_DCHECK(render_thread_.CalledOnValidThread());
   const StreamConfig reverse_config = {
       rev_sample_rate_hz, ChannelsFromLayout(layout), LayoutHasKeyboard(layout),
   };
@@ -733,6 +743,7 @@ int AudioProcessingImpl::ProcessReverseStream(
     const StreamConfig& reverse_input_config,
     const StreamConfig& reverse_output_config,
     float* const* dest) {
+  RTC_DCHECK(render_thread_.CalledOnValidThread());
   RETURN_ON_ERR(
       AnalyzeReverseStream(src, reverse_input_config, reverse_output_config));
   if (is_rev_processed()) {
@@ -791,6 +802,7 @@ int AudioProcessingImpl::AnalyzeReverseStream(
 }
 
 int AudioProcessingImpl::ProcessReverseStream(AudioFrame* frame) {
+  RTC_DCHECK(render_thread_.CalledOnValidThread());
   RETURN_ON_ERR(AnalyzeReverseStream(frame));
   if (is_rev_processed()) {
     render_audio_->InterleaveTo(frame, true);
@@ -800,6 +812,7 @@ int AudioProcessingImpl::ProcessReverseStream(AudioFrame* frame) {
 }
 
 int AudioProcessingImpl::AnalyzeReverseStream(AudioFrame* frame) {
+  RTC_DCHECK(render_thread_.CalledOnValidThread());
   CriticalSectionScoped crit_scoped(crit_);
   if (frame == NULL) {
     return kNullPointerError;
@@ -877,6 +890,7 @@ int AudioProcessingImpl::ProcessReverseStreamLocked() {
 }
 
 int AudioProcessingImpl::set_stream_delay_ms(int delay) {
+  RTC_DCHECK(capture_thread_.CalledOnValidThread());
   Error retval = kNoError;
   was_stream_delay_set_ = true;
   delay += delay_offset_ms_;
@@ -897,6 +911,7 @@ int AudioProcessingImpl::set_stream_delay_ms(int delay) {
 }
 
 int AudioProcessingImpl::stream_delay_ms() const {
+  RTC_DCHECK(capture_thread_.CalledOnValidThread());
   return stream_delay_ms_;
 }
 
@@ -905,15 +920,18 @@ bool AudioProcessingImpl::was_stream_delay_set() const {
 }
 
 void AudioProcessingImpl::set_stream_key_pressed(bool key_pressed) {
+  RTC_DCHECK(capture_thread_.CalledOnValidThread());
   key_pressed_ = key_pressed;
 }
 
 void AudioProcessingImpl::set_delay_offset_ms(int offset) {
+  RTC_DCHECK(capture_thread_.CalledOnValidThread());
   CriticalSectionScoped crit_scoped(crit_);
   delay_offset_ms_ = offset;
 }
 
 int AudioProcessingImpl::delay_offset_ms() const {
+  RTC_DCHECK(capture_thread_.CalledOnValidThread());
   return delay_offset_ms_;
 }