Bug 904784: use a separate critical section for the recording callback r=mwu
--- a/media/webrtc/trunk/webrtc/modules/audio_device/audio_device_opensles.cc
+++ b/media/webrtc/trunk/webrtc/modules/audio_device/audio_device_opensles.cc
@@ -31,16 +31,17 @@
#define WEBRTC_OPENSL_TRACE WEBRTC_TRACE
#endif
namespace webrtc {
AudioDeviceAndroidOpenSLES::AudioDeviceAndroidOpenSLES(const int32_t id)
: voe_audio_buffer_(NULL),
crit_sect_(*CriticalSectionWrapper::CreateCriticalSection()),
+ callback_crit_sect_(*CriticalSectionWrapper::CreateCriticalSection()),
id_(id),
sles_engine_(NULL),
sles_player_(NULL),
sles_engine_itf_(NULL),
sles_player_itf_(NULL),
sles_player_sbq_itf_(NULL),
sles_output_mixer_(NULL),
sles_speaker_volume_(NULL),
@@ -271,16 +272,17 @@ int32_t AudioDeviceAndroidOpenSLES::Micr
bool& available) {
// We always assume it's available.
available = true;
return 0;
}
int32_t AudioDeviceAndroidOpenSLES::InitMicrophone() {
CriticalSectionScoped lock(&crit_sect_);
+ CriticalSectionScoped callback_lock(&callback_crit_sect_);
if (is_recording_) {
WEBRTC_OPENSL_TRACE(kTraceWarning, kTraceAudioDevice, id_,
" Recording already started");
return -1;
}
if (!is_recording_dev_specified_) {
WEBRTC_OPENSL_TRACE(kTraceError, kTraceAudioDevice, id_,
" Recording device is not specified");
@@ -935,16 +937,17 @@ int32_t AudioDeviceAndroidOpenSLES::Init
}
is_rec_initialized_ = true;
return 0;
}
int32_t AudioDeviceAndroidOpenSLES::StartRecording() {
CriticalSectionScoped lock(&crit_sect_);
+ CriticalSectionScoped callback_lock(&callback_crit_sect_);
if (!is_rec_initialized_) {
WEBRTC_OPENSL_TRACE(kTraceError, kTraceAudioDevice, id_,
" Recording not initialized");
return -1;
}
if (is_recording_) {
@@ -1074,29 +1077,31 @@ int32_t AudioDeviceAndroidOpenSLES::Stop
} else {
WEBRTC_TRACE(kTraceError, kTraceAudioDevice, id_,
"Failed to stop recording thread ");
return -1;
}
}
CriticalSectionScoped lock(&crit_sect_);
+ CriticalSectionScoped callback_lock(&callback_crit_sect_);
is_rec_initialized_ = false;
is_recording_ = false;
rec_warning_ = 0;
rec_error_ = 0;
return 0;
}
bool AudioDeviceAndroidOpenSLES::RecordingIsInitialized() const {
return is_rec_initialized_;
}
bool AudioDeviceAndroidOpenSLES::Recording() const {
+ CriticalSectionScoped callback_lock(&callback_crit_sect_);
return is_recording_;
}
bool AudioDeviceAndroidOpenSLES::PlayoutIsInitialized() const {
return is_play_initialized_;
}
int32_t AudioDeviceAndroidOpenSLES::StartPlayout() {
@@ -1361,50 +1366,59 @@ bool AudioDeviceAndroidOpenSLES::RecThre
// TODO(leozwang): Add seting correct scheduling and thread priority.
const unsigned int num_samples = mic_sampling_rate_ / 100;
const unsigned int num_bytes =
N_REC_CHANNELS * num_samples * sizeof(int16_t);
const unsigned int total_bytes = num_bytes;
int8_t buf[REC_MAX_TEMP_BUF_SIZE_PER_10ms];
+ // Always grab crit_sect_ first, then callback_crit_sect_
+ // And vice-versa for releasing
crit_sect_.Enter();
+ callback_crit_sect_.Enter();
while (is_recording_) {
if (rec_voe_audio_queue_.size() <= 0) {
+ callback_crit_sect_.Leave();
crit_sect_.Leave();
// Wait for max 40ms for incoming audio data before looping the
// poll and checking for ::Stop() being called (which waits for us
// to return). Actual time between audio callbacks will vary, but
// is based on AudioRecord min buffer sizes, which may be 15-80ms
// or more, depending on sampling frequency, but typically will be
// 25ish ms at 44100Hz. We also don't want to take "too long" to
// exit after ::Stop(). This value of 40ms is arbitrary.
rec_timer_.Wait(40);
crit_sect_.Enter();
+ callback_crit_sect_.Enter();
if (rec_voe_audio_queue_.size() <= 0) {
// still no audio data; check for ::Stop()
+ callback_crit_sect_.Leave();
crit_sect_.Leave();
return true;
}
}
int8_t* audio = rec_voe_audio_queue_.front();
rec_voe_audio_queue_.pop();
memcpy(buf, audio, total_bytes);
memset(audio, 0, total_bytes);
rec_voe_ready_queue_.push(audio);
voe_audio_buffer_->SetRecordedBuffer(buf, num_samples);
voe_audio_buffer_->SetVQEData(playout_delay_, recording_delay_, 0);
// All other implementations UnLock around DeliverRecordedData() only
+ callback_crit_sect_.Leave();
crit_sect_.Leave();
voe_audio_buffer_->DeliverRecordedData();
crit_sect_.Enter();
+ callback_crit_sect_.Enter();
}
+ callback_crit_sect_.Leave();
crit_sect_.Leave();
// if is_recording is false, we either *just* were started, or for some reason
// Stop() failed to get us to return for 10 seconds - and when we return here,
// Stop will kill us anyways.
rec_timer_.Wait(1);
return true;
@@ -1415,17 +1429,20 @@ void AudioDeviceAndroidOpenSLES::Recorde
const unsigned int num_samples = mic_sampling_rate_ / 100;
const unsigned int num_bytes =
N_REC_CHANNELS * num_samples * sizeof(int16_t);
const unsigned int total_bytes = num_bytes;
int8_t* audio;
{
- CriticalSectionScoped lock(&crit_sect_);
+ // use this instead of crit_sect_ to avoid race against StopRecording()
+ // (which holds crit_sect, and then waits for the thread that calls
+ // this to exit, leading to possible deadlock (bug 904784)
+ CriticalSectionScoped lock(&callback_crit_sect_);
if (!is_recording_) {
return;
}
audio = rec_queue_.front();
rec_queue_.pop();
rec_voe_audio_queue_.push(audio);
rec_timer_.Set(); // wake up record thread to process it
--- a/media/webrtc/trunk/webrtc/modules/audio_device/audio_device_opensles.h
+++ b/media/webrtc/trunk/webrtc/modules/audio_device/audio_device_opensles.h
@@ -235,16 +235,20 @@ class AudioDeviceAndroidOpenSLES: public
void UpdatePlayoutDelay(uint32_t nSamplePlayed);
// Init
int32_t InitSampleRate();
// Misc
AudioDeviceBuffer* voe_audio_buffer_;
CriticalSectionWrapper& crit_sect_;
+ // callback_crit_sect_ is used to lock rec_queue and rec_voe_ready_queue
+ // and also for changing is_recording to false. If you hold this and
+ // crit_sect_, you must grab crit_sect_ first
+ CriticalSectionWrapper& callback_crit_sect_;
int32_t id_;
// audio unit
SLObjectItf sles_engine_;
// playout device
SLObjectItf sles_player_;
SLEngineItf sles_engine_itf_;