From 920d30bc743b76d30c6cae7ad4c97af2e6c24e4c Mon Sep 17 00:00:00 2001 From: aleloi Date: Thu, 20 Oct 2016 14:23:24 -0700 Subject: [PATCH] Replaced thread checker with race checker in AudioMixer. This change is due to an incorrect understanding of the threading model in Chrome. The new AudioMixer has a thread checker to ensure that mixing is always done from a single thread. Mixing is done on the Audio Output Thread. When run in Chrome, it can change. Even if the thread changes, there is never more than one audio thread, and mixing is done sequentially. The threading checks and variable access checks are replaced with rtc::RaceChecker counterparts. NOTRY=True BUG=webrtc:6346 Review-Url: https://codereview.webrtc.org/2437913003 Cr-Commit-Position: refs/heads/master@{#14712} --- .../modules/audio_mixer/audio_mixer_impl.cc | 13 ++++++------ webrtc/modules/audio_mixer/audio_mixer_impl.h | 20 +++++++++---------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/webrtc/modules/audio_mixer/audio_mixer_impl.cc b/webrtc/modules/audio_mixer/audio_mixer_impl.cc index 57fae8cc0d..3fcd3e3c66 100644 --- a/webrtc/modules/audio_mixer/audio_mixer_impl.cc +++ b/webrtc/modules/audio_mixer/audio_mixer_impl.cc @@ -143,7 +143,6 @@ AudioMixerImpl::AudioMixerImpl(std::unique_ptr limiter) time_stamp_(0), limiter_(std::move(limiter)) { SetOutputFrequency(kDefaultFrequency); - thread_checker_.DetachFromThread(); } AudioMixerImpl::~AudioMixerImpl() {} @@ -189,7 +188,7 @@ void AudioMixerImpl::Mix(int sample_rate, size_t number_of_channels, AudioFrame* audio_frame_for_mixing) { RTC_DCHECK(number_of_channels == 1 || number_of_channels == 2); - RTC_DCHECK_RUN_ON(&thread_checker_); + RTC_DCHECK_RUNS_SERIALIZED(&race_checker_); if (OutputFrequency() != sample_rate) { SetOutputFrequency(sample_rate); @@ -229,13 +228,13 @@ void AudioMixerImpl::Mix(int sample_rate, } void AudioMixerImpl::SetOutputFrequency(int frequency) { - RTC_DCHECK_RUN_ON(&thread_checker_); + RTC_DCHECK_RUNS_SERIALIZED(&race_checker_); output_frequency_ = frequency; sample_size_ = (output_frequency_ * kFrameDurationInMs) / 1000; } int AudioMixerImpl::OutputFrequency() const { - RTC_DCHECK_RUN_ON(&thread_checker_); + RTC_DCHECK_RUNS_SERIALIZED(&race_checker_); return output_frequency_; } @@ -259,7 +258,7 @@ bool AudioMixerImpl::RemoveSource(Source* audio_source) { } AudioFrameList AudioMixerImpl::GetAudioFromSources() { - RTC_DCHECK_RUN_ON(&thread_checker_); + RTC_DCHECK_RUNS_SERIALIZED(&race_checker_); AudioFrameList result; std::vector audio_source_mixing_data_list; std::vector ramp_list; @@ -312,7 +311,7 @@ AudioFrameList AudioMixerImpl::GetAudioFromSources() { bool AudioMixerImpl::LimitMixedAudio(AudioFrame* mixed_audio) const { - RTC_DCHECK_RUN_ON(&thread_checker_); + RTC_DCHECK_RUNS_SERIALIZED(&race_checker_); if (!use_limiter_) { return true; } @@ -342,7 +341,7 @@ bool AudioMixerImpl::LimitMixedAudio(AudioFrame* mixed_audio) const { bool AudioMixerImpl::GetAudioSourceMixabilityStatusForTest( AudioMixerImpl::Source* audio_source) const { - RTC_DCHECK_RUN_ON(&thread_checker_); + RTC_DCHECK_RUNS_SERIALIZED(&race_checker_); rtc::CritScope lock(&crit_); const auto non_anonymous_iter = diff --git a/webrtc/modules/audio_mixer/audio_mixer_impl.h b/webrtc/modules/audio_mixer/audio_mixer_impl.h index 500bb7862e..e011e8a340 100644 --- a/webrtc/modules/audio_mixer/audio_mixer_impl.h +++ b/webrtc/modules/audio_mixer/audio_mixer_impl.h @@ -17,7 +17,7 @@ #include "webrtc/api/audio/audio_mixer.h" #include "webrtc/base/scoped_ref_ptr.h" #include "webrtc/base/thread_annotations.h" -#include "webrtc/base/thread_checker.h" +#include "webrtc/base/race_checker.h" #include "webrtc/modules/audio_processing/include/audio_processing.h" #include "webrtc/modules/include/module_common_types.h" #include "webrtc/system_wrappers/include/critical_section_wrapper.h" @@ -83,27 +83,27 @@ class AudioMixerImpl : public AudioMixer { bool LimitMixedAudio(AudioFrame* mixed_audio) const; - + // The critical section lock guards audio source insertion and + // removal, which can be done from any thread. The race checker + // checks that mixing is done sequentially. rtc::CriticalSection crit_; + rtc::RaceChecker race_checker_; // The current sample frequency and sample size when mixing. - int output_frequency_ ACCESS_ON(&thread_checker_); - size_t sample_size_ ACCESS_ON(&thread_checker_); + int output_frequency_ GUARDED_BY(race_checker_); + size_t sample_size_ GUARDED_BY(race_checker_); // List of all audio sources. Note all lists are disjunct SourceStatusList audio_source_list_ GUARDED_BY(crit_); // May be mixed. // Determines if we will use a limiter for clipping protection during // mixing. - bool use_limiter_ ACCESS_ON(&thread_checker_); + bool use_limiter_ GUARDED_BY(race_checker_); - uint32_t time_stamp_ ACCESS_ON(&thread_checker_); - - // Ensures that Mix is called from the same thread. - rtc::ThreadChecker thread_checker_; + uint32_t time_stamp_ GUARDED_BY(race_checker_); // Used for inhibiting saturation in mixing. - std::unique_ptr limiter_ ACCESS_ON(&thread_checker_); + std::unique_ptr limiter_ GUARDED_BY(race_checker_); RTC_DISALLOW_COPY_AND_ASSIGN(AudioMixerImpl); };