Add CriticalSection to fakeaudiocapturemodule to protect the variables which will be accessed from process_thread_ and the main thread.
TEST=try bots
BUG=1205
R=henrike@webrtc.org, kjellander@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/2419004
git-svn-id: http://webrtc.googlecode.com/svn/trunk@5019 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/talk/app/webrtc/test/fakeaudiocapturemodule.cc b/talk/app/webrtc/test/fakeaudiocapturemodule.cc
index 4bdaf89..3b36163 100644
--- a/talk/app/webrtc/test/fakeaudiocapturemodule.cc
+++ b/talk/app/webrtc/test/fakeaudiocapturemodule.cc
@@ -52,6 +52,7 @@
static const uint32_t kMaxVolume = 14392;
enum {
+ MSG_START_PROCESS,
MSG_RUN_PROCESS,
MSG_STOP_PROCESS,
};
@@ -89,6 +90,7 @@
}
int FakeAudioCaptureModule::frames_received() const {
+ talk_base::CritScope cs(&crit_);
return frames_received_;
}
@@ -142,6 +144,7 @@
int32_t FakeAudioCaptureModule::RegisterAudioCallback(
webrtc::AudioTransport* audio_callback) {
+ talk_base::CritScope cs(&crit_callback_);
audio_callback_ = audio_callback;
return 0;
}
@@ -245,18 +248,28 @@
if (!play_is_initialized_) {
return -1;
}
- playing_ = true;
- UpdateProcessing();
+ {
+ talk_base::CritScope cs(&crit_);
+ playing_ = true;
+ }
+ bool start = true;
+ UpdateProcessing(start);
return 0;
}
int32_t FakeAudioCaptureModule::StopPlayout() {
- playing_ = false;
- UpdateProcessing();
+ bool start = false;
+ {
+ talk_base::CritScope cs(&crit_);
+ playing_ = false;
+ start = ShouldStartProcessing();
+ }
+ UpdateProcessing(start);
return 0;
}
bool FakeAudioCaptureModule::Playing() const {
+ talk_base::CritScope cs(&crit_);
return playing_;
}
@@ -264,18 +277,28 @@
if (!rec_is_initialized_) {
return -1;
}
- recording_ = true;
- UpdateProcessing();
+ {
+ talk_base::CritScope cs(&crit_);
+ recording_ = true;
+ }
+ bool start = true;
+ UpdateProcessing(start);
return 0;
}
int32_t FakeAudioCaptureModule::StopRecording() {
- recording_ = false;
- UpdateProcessing();
+ bool start = false;
+ {
+ talk_base::CritScope cs(&crit_);
+ recording_ = false;
+ start = ShouldStartProcessing();
+ }
+ UpdateProcessing(start);
return 0;
}
bool FakeAudioCaptureModule::Recording() const {
+ talk_base::CritScope cs(&crit_);
return recording_;
}
@@ -373,12 +396,14 @@
return 0;
}
-int32_t FakeAudioCaptureModule::SetMicrophoneVolume(uint32_t /*volume*/) {
- ASSERT(false);
+int32_t FakeAudioCaptureModule::SetMicrophoneVolume(uint32_t volume) {
+ talk_base::CritScope cs(&crit_);
+ current_mic_level_ = volume;
return 0;
}
int32_t FakeAudioCaptureModule::MicrophoneVolume(uint32_t* volume) const {
+ talk_base::CritScope cs(&crit_);
*volume = current_mic_level_;
return 0;
}
@@ -594,6 +619,9 @@
void FakeAudioCaptureModule::OnMessage(talk_base::Message* msg) {
switch (msg->message_id) {
+ case MSG_START_PROCESS:
+ StartProcessP();
+ break;
case MSG_RUN_PROCESS:
ProcessFrameP();
break;
@@ -640,33 +668,48 @@
return false;
}
-void FakeAudioCaptureModule::UpdateProcessing() {
- const bool process = recording_ || playing_;
- if (process) {
- if (started_) {
- // Already started.
- return;
- }
- process_thread_->Post(this, MSG_RUN_PROCESS);
+bool FakeAudioCaptureModule::ShouldStartProcessing() {
+ return recording_ || playing_;
+}
+
+void FakeAudioCaptureModule::UpdateProcessing(bool start) {
+ if (start) {
+ process_thread_->Post(this, MSG_START_PROCESS);
} else {
process_thread_->Send(this, MSG_STOP_PROCESS);
}
}
+void FakeAudioCaptureModule::StartProcessP() {
+ ASSERT(talk_base::Thread::Current() == process_thread_);
+ if (started_) {
+ // Already started.
+ return;
+ }
+ ProcessFrameP();
+}
+
void FakeAudioCaptureModule::ProcessFrameP() {
ASSERT(talk_base::Thread::Current() == process_thread_);
if (!started_) {
next_frame_time_ = talk_base::Time();
started_ = true;
}
+
+ bool playing;
+ bool recording;
+ {
+ talk_base::CritScope cs(&crit_);
+ playing = playing_;
+ recording = recording_;
+ }
+
// Receive and send frames every kTimePerFrameMs.
- if (audio_callback_ != NULL) {
- if (playing_) {
- ReceiveFrameP();
- }
- if (recording_) {
- SendFrameP();
- }
+ if (playing) {
+ ReceiveFrameP();
+ }
+ if (recording) {
+ SendFrameP();
}
next_frame_time_ += kTimePerFrameMs;
@@ -678,35 +721,51 @@
void FakeAudioCaptureModule::ReceiveFrameP() {
ASSERT(talk_base::Thread::Current() == process_thread_);
- ResetRecBuffer();
- uint32_t nSamplesOut = 0;
- if (audio_callback_->NeedMorePlayData(kNumberSamples, kNumberBytesPerSample,
- kNumberOfChannels, kSamplesPerSecond,
- rec_buffer_, nSamplesOut) != 0) {
- ASSERT(false);
+ {
+ talk_base::CritScope cs(&crit_callback_);
+ if (!audio_callback_) {
+ return;
+ }
+ ResetRecBuffer();
+ uint32_t nSamplesOut = 0;
+ if (audio_callback_->NeedMorePlayData(kNumberSamples, kNumberBytesPerSample,
+ kNumberOfChannels, kSamplesPerSecond,
+ rec_buffer_, nSamplesOut) != 0) {
+ ASSERT(false);
+ }
+ ASSERT(nSamplesOut == kNumberSamples);
}
- ASSERT(nSamplesOut == kNumberSamples);
// The SetBuffer() function ensures that after decoding, the audio buffer
// should contain samples of similar magnitude (there is likely to be some
// distortion due to the audio pipeline). If one sample is detected to
// have the same or greater magnitude somewhere in the frame, an actual frame
// has been received from the remote side (i.e. faked frames are not being
// pulled).
- if (CheckRecBuffer(kHighSampleValue)) ++frames_received_;
+ if (CheckRecBuffer(kHighSampleValue)) {
+ talk_base::CritScope cs(&crit_);
+ ++frames_received_;
+ }
}
void FakeAudioCaptureModule::SendFrameP() {
ASSERT(talk_base::Thread::Current() == process_thread_);
+ talk_base::CritScope cs(&crit_callback_);
+ if (!audio_callback_) {
+ return;
+ }
bool key_pressed = false;
+ uint32_t current_mic_level = 0;
+ MicrophoneVolume(¤t_mic_level);
if (audio_callback_->RecordedDataIsAvailable(send_buffer_, kNumberSamples,
kNumberBytesPerSample,
kNumberOfChannels,
kSamplesPerSecond, kTotalDelayMs,
- kClockDriftMs, current_mic_level_,
+ kClockDriftMs, current_mic_level,
key_pressed,
- current_mic_level_) != 0) {
+ current_mic_level) != 0) {
ASSERT(false);
}
+ SetMicrophoneVolume(current_mic_level);
}
void FakeAudioCaptureModule::StopProcessP() {
diff --git a/talk/app/webrtc/test/fakeaudiocapturemodule.h b/talk/app/webrtc/test/fakeaudiocapturemodule.h
index c32fa1f..2267902 100644
--- a/talk/app/webrtc/test/fakeaudiocapturemodule.h
+++ b/talk/app/webrtc/test/fakeaudiocapturemodule.h
@@ -38,6 +38,7 @@
#define TALK_APP_WEBRTC_TEST_FAKEAUDIOCAPTUREMODULE_H_
#include "talk/base/basictypes.h"
+#include "talk/base/criticalsection.h"
#include "talk/base/messagehandler.h"
#include "talk/base/scoped_ref_ptr.h"
#include "webrtc/common_types.h"
@@ -88,6 +89,7 @@
virtual int32_t RegisterEventObserver(
webrtc::AudioDeviceObserver* event_callback);
+ // Note: Calling this method from a callback may result in deadlock.
virtual int32_t RegisterAudioCallback(webrtc::AudioTransport* audio_callback);
virtual int32_t Init();
@@ -225,10 +227,15 @@
// equal to |value|.
bool CheckRecBuffer(int value);
- // Starts or stops the pushing and pulling of audio frames depending on if
- // recording or playback has been enabled/started.
- void UpdateProcessing();
+ // Returns true/false depending on if recording or playback has been
+ // enabled/started.
+ bool ShouldStartProcessing();
+ // Starts or stops the pushing and pulling of audio frames.
+ void UpdateProcessing(bool start);
+
+ // Starts the periodic calling of ProcessFrame() in a thread safe way.
+ void StartProcessP();
// Periodcally called function that ensures that frames are pulled and pushed
// periodically if enabled/started.
void ProcessFrameP();
@@ -275,6 +282,13 @@
// indicate that the frames are not faked somewhere in the audio pipeline
// (e.g. by a jitter buffer).
int frames_received_;
+
+ // Protects variables that are accessed from process_thread_ and
+ // the main thread.
+ mutable talk_base::CriticalSection crit_;
+ // Protects |audio_callback_| that is accessed from process_thread_ and
+ // the main thread.
+ talk_base::CriticalSection crit_callback_;
};
#endif // TALK_APP_WEBRTC_TEST_FAKEAUDIOCAPTUREMODULE_H_
diff --git a/tools/valgrind-webrtc/tsan_v2/suppressions.txt b/tools/valgrind-webrtc/tsan_v2/suppressions.txt
index 54ea84e..32a41b8 100644
--- a/tools/valgrind-webrtc/tsan_v2/suppressions.txt
+++ b/tools/valgrind-webrtc/tsan_v2/suppressions.txt
@@ -11,10 +11,6 @@
# See https://code.google.com/p/webrtc/issues/detail?id=2484 for details.
# race:webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc
-# libjingle_peerconnection_unittest
-# See https://code.google.com/p/webrtc/issues/detail?id=1205
-race:talk/app/webrtc/test/fakeaudiocapturemodule.cc
-
# libjingle_p2p_unittest
# See https://code.google.com/p/webrtc/issues/detail?id=2079
race:talk/base/messagequeue.cc
