Elimiteted race condition in the AudioMixer.
The mixer allocates an audio frame for each added data source. This audio frame was deallocated when a source was removed from the mixer. Source removal could happen during the mixing, and the existing locking scheme (and the Clang thread checker) was not sufficient to prevent a data race. After this change, the mixer doesn't release its lock until it is finished with the sources' Audio frames. Since multi-threaded access to the mixer only happens when a source is added or removed, we believe that this change wouldn't have any noticeable performance impact. NOTRY=True BUG=webrtc:6346 Review-Url: https://codereview.webrtc.org/2439283002 Cr-Commit-Position: refs/heads/master@{#14744}
This commit is contained in:
@ -200,23 +200,23 @@ void AudioMixerImpl::Mix(int sample_rate,
|
||||
{
|
||||
rtc::CritScope lock(&crit_);
|
||||
mix_list = GetAudioFromSources();
|
||||
|
||||
for (const auto& frame : mix_list) {
|
||||
RemixFrame(number_of_channels, frame);
|
||||
}
|
||||
|
||||
audio_frame_for_mixing->UpdateFrame(
|
||||
-1, time_stamp_, NULL, 0, OutputFrequency(), AudioFrame::kNormalSpeech,
|
||||
AudioFrame::kVadPassive, number_of_channels);
|
||||
|
||||
time_stamp_ += static_cast<uint32_t>(sample_size_);
|
||||
|
||||
use_limiter_ = mix_list.size() > 1;
|
||||
|
||||
// We only use the limiter if we're actually mixing multiple streams.
|
||||
MixFromList(audio_frame_for_mixing, mix_list, use_limiter_);
|
||||
}
|
||||
|
||||
for (const auto& frame : mix_list) {
|
||||
RemixFrame(number_of_channels, frame);
|
||||
}
|
||||
|
||||
audio_frame_for_mixing->UpdateFrame(
|
||||
-1, time_stamp_, NULL, 0, OutputFrequency(), AudioFrame::kNormalSpeech,
|
||||
AudioFrame::kVadPassive, number_of_channels);
|
||||
|
||||
time_stamp_ += static_cast<uint32_t>(sample_size_);
|
||||
|
||||
use_limiter_ = mix_list.size() > 1;
|
||||
|
||||
// We only use the limiter if we're actually mixing multiple streams.
|
||||
MixFromList(audio_frame_for_mixing, mix_list, use_limiter_);
|
||||
|
||||
if (audio_frame_for_mixing->samples_per_channel_ == 0) {
|
||||
// Nothing was mixed, set the audio samples to silence.
|
||||
audio_frame_for_mixing->samples_per_channel_ = sample_size_;
|
||||
|
||||
@ -57,7 +57,7 @@ class AudioMixerImpl : public AudioMixer {
|
||||
|
||||
void Mix(int sample_rate_hz,
|
||||
size_t number_of_channels,
|
||||
AudioFrame* audio_frame_for_mixing) override;
|
||||
AudioFrame* audio_frame_for_mixing) override LOCKS_EXCLUDED(crit_);
|
||||
|
||||
// Returns true if the source was mixed last round. Returns
|
||||
// false and logs an error if the source was never added to the
|
||||
|
||||
Reference in New Issue
Block a user