FakeAudioCaptureModule: remove lock recursions.
This change removes lock recursions and adds thread annotations.
The module had incorrect locking WRT the callback critical section:
ProcessFrameP: locks crit_
ReceiveFrameP: locks crit_callback_
-------------
SendFrameP: locks crit_callback_
MicrophoneVolume: locks crit_
Lock crit_callback_ was rolled in under crit_ instead.
Bug: webrtc:11567
Change-Id: I974fe91d44de0ddf1a1287fe91db9dfe63a61af9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175662
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Commit-Queue: Markus Handell <handellm@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31313}
diff --git a/pc/BUILD.gn b/pc/BUILD.gn
index f553f15..c5223b1 100644
--- a/pc/BUILD.gn
+++ b/pc/BUILD.gn
@@ -499,6 +499,7 @@
"../rtc_base:rtc_base_approved",
"../rtc_base:rtc_task_queue",
"../rtc_base:task_queue_for_test",
+ "../rtc_base/synchronization:sequence_checker",
"../rtc_base/task_utils:repeating_task",
"../rtc_base/third_party/sigslot",
"../test:test_support",
diff --git a/pc/test/fake_audio_capture_module.cc b/pc/test/fake_audio_capture_module.cc
index db0886d..1a7efd4 100644
--- a/pc/test/fake_audio_capture_module.cc
+++ b/pc/test/fake_audio_capture_module.cc
@@ -47,7 +47,9 @@
current_mic_level_(kMaxVolume),
started_(false),
next_frame_time_(0),
- frames_received_(0) {}
+ frames_received_(0) {
+ process_thread_checker_.Detach();
+}
FakeAudioCaptureModule::~FakeAudioCaptureModule() {
if (process_thread_) {
@@ -77,7 +79,7 @@
int32_t FakeAudioCaptureModule::RegisterAudioCallback(
webrtc::AudioTransport* audio_callback) {
- rtc::CritScope cs(&crit_callback_);
+ rtc::CritScope cs(&crit_);
audio_callback_ = audio_callback;
return 0;
}
@@ -448,29 +450,34 @@
if (process_thread_) {
process_thread_->Stop();
process_thread_.reset(nullptr);
+ process_thread_checker_.Detach();
}
+ rtc::CritScope lock(&crit_);
started_ = false;
}
}
void FakeAudioCaptureModule::StartProcessP() {
- RTC_CHECK(process_thread_->IsCurrent());
- if (started_) {
- // Already started.
- return;
+ RTC_DCHECK_RUN_ON(&process_thread_checker_);
+ {
+ rtc::CritScope lock(&crit_);
+ if (started_) {
+ // Already started.
+ return;
+ }
}
ProcessFrameP();
}
void FakeAudioCaptureModule::ProcessFrameP() {
- RTC_CHECK(process_thread_->IsCurrent());
- if (!started_) {
- next_frame_time_ = rtc::TimeMillis();
- started_ = true;
- }
-
+ RTC_DCHECK_RUN_ON(&process_thread_checker_);
{
rtc::CritScope cs(&crit_);
+ if (!started_) {
+ next_frame_time_ = rtc::TimeMillis();
+ started_ = true;
+ }
+
// Receive and send frames every kTimePerFrameMs.
if (playing_) {
ReceiveFrameP();
@@ -488,24 +495,22 @@
}
void FakeAudioCaptureModule::ReceiveFrameP() {
- RTC_CHECK(process_thread_->IsCurrent());
- {
- rtc::CritScope cs(&crit_callback_);
- if (!audio_callback_) {
- return;
- }
- ResetRecBuffer();
- size_t nSamplesOut = 0;
- int64_t elapsed_time_ms = 0;
- int64_t ntp_time_ms = 0;
- if (audio_callback_->NeedMorePlayData(
- kNumberSamples, kNumberBytesPerSample, kNumberOfChannels,
- kSamplesPerSecond, rec_buffer_, nSamplesOut, &elapsed_time_ms,
- &ntp_time_ms) != 0) {
- RTC_NOTREACHED();
- }
- RTC_CHECK(nSamplesOut == kNumberSamples);
+ RTC_DCHECK_RUN_ON(&process_thread_checker_);
+ if (!audio_callback_) {
+ return;
}
+ ResetRecBuffer();
+ size_t nSamplesOut = 0;
+ int64_t elapsed_time_ms = 0;
+ int64_t ntp_time_ms = 0;
+ if (audio_callback_->NeedMorePlayData(kNumberSamples, kNumberBytesPerSample,
+ kNumberOfChannels, kSamplesPerSecond,
+ rec_buffer_, nSamplesOut,
+ &elapsed_time_ms, &ntp_time_ms) != 0) {
+ RTC_NOTREACHED();
+ }
+ RTC_CHECK(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
@@ -513,25 +518,22 @@
// has been received from the remote side (i.e. faked frames are not being
// pulled).
if (CheckRecBuffer(kHighSampleValue)) {
- rtc::CritScope cs(&crit_);
++frames_received_;
}
}
void FakeAudioCaptureModule::SendFrameP() {
- RTC_CHECK(process_thread_->IsCurrent());
- rtc::CritScope cs(&crit_callback_);
+ RTC_DCHECK_RUN_ON(&process_thread_checker_);
if (!audio_callback_) {
return;
}
bool key_pressed = false;
- uint32_t current_mic_level = 0;
- MicrophoneVolume(¤t_mic_level);
+ uint32_t current_mic_level = current_mic_level_;
if (audio_callback_->RecordedDataIsAvailable(
send_buffer_, kNumberSamples, kNumberBytesPerSample,
kNumberOfChannels, kSamplesPerSecond, kTotalDelayMs, kClockDriftMs,
current_mic_level, key_pressed, current_mic_level) != 0) {
RTC_NOTREACHED();
}
- SetMicrophoneVolume(current_mic_level);
+ current_mic_level_ = current_mic_level;
}
diff --git a/pc/test/fake_audio_capture_module.h b/pc/test/fake_audio_capture_module.h
index 0af3810..498b6da 100644
--- a/pc/test/fake_audio_capture_module.h
+++ b/pc/test/fake_audio_capture_module.h
@@ -26,6 +26,7 @@
#include "modules/audio_device/include/audio_device.h"
#include "rtc_base/critical_section.h"
#include "rtc_base/message_handler.h"
+#include "rtc_base/synchronization/sequence_checker.h"
namespace rtc {
class Thread;
@@ -47,13 +48,13 @@
// Returns the number of frames that have been successfully pulled by the
// instance. Note that correctly detecting success can only be done if the
// pulled frame was generated/pushed from a FakeAudioCaptureModule.
- int frames_received() const;
+ int frames_received() const RTC_LOCKS_EXCLUDED(crit_);
int32_t ActiveAudioLayer(AudioLayer* audio_layer) const override;
// Note: Calling this method from a callback may result in deadlock.
- int32_t RegisterAudioCallback(
- webrtc::AudioTransport* audio_callback) override;
+ int32_t RegisterAudioCallback(webrtc::AudioTransport* audio_callback) override
+ RTC_LOCKS_EXCLUDED(crit_);
int32_t Init() override;
int32_t Terminate() override;
@@ -80,12 +81,12 @@
int32_t InitRecording() override;
bool RecordingIsInitialized() const override;
- int32_t StartPlayout() override;
- int32_t StopPlayout() override;
- bool Playing() const override;
- int32_t StartRecording() override;
- int32_t StopRecording() override;
- bool Recording() const override;
+ int32_t StartPlayout() RTC_LOCKS_EXCLUDED(crit_) override;
+ int32_t StopPlayout() RTC_LOCKS_EXCLUDED(crit_) override;
+ bool Playing() const RTC_LOCKS_EXCLUDED(crit_) override;
+ int32_t StartRecording() RTC_LOCKS_EXCLUDED(crit_) override;
+ int32_t StopRecording() RTC_LOCKS_EXCLUDED(crit_) override;
+ bool Recording() const RTC_LOCKS_EXCLUDED(crit_) override;
int32_t InitSpeaker() override;
bool SpeakerIsInitialized() const override;
@@ -99,8 +100,10 @@
int32_t MinSpeakerVolume(uint32_t* min_volume) const override;
int32_t MicrophoneVolumeIsAvailable(bool* available) override;
- int32_t SetMicrophoneVolume(uint32_t volume) override;
- int32_t MicrophoneVolume(uint32_t* volume) const override;
+ int32_t SetMicrophoneVolume(uint32_t volume)
+ RTC_LOCKS_EXCLUDED(crit_) override;
+ int32_t MicrophoneVolume(uint32_t* volume) const
+ RTC_LOCKS_EXCLUDED(crit_) override;
int32_t MaxMicrophoneVolume(uint32_t* max_volume) const override;
int32_t MinMicrophoneVolume(uint32_t* min_volume) const override;
@@ -170,26 +173,28 @@
// Returns true/false depending on if recording or playback has been
// enabled/started.
- bool ShouldStartProcessing();
+ bool ShouldStartProcessing() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_);
// Starts or stops the pushing and pulling of audio frames.
- void UpdateProcessing(bool start);
+ void UpdateProcessing(bool start) RTC_LOCKS_EXCLUDED(crit_);
// 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();
+ void ProcessFrameP() RTC_LOCKS_EXCLUDED(crit_);
// Pulls frames from the registered webrtc::AudioTransport.
- void ReceiveFrameP();
+ void ReceiveFrameP() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_);
// Pushes frames to the registered webrtc::AudioTransport.
- void SendFrameP();
+ void SendFrameP() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_);
// Callback for playout and recording.
- webrtc::AudioTransport* audio_callback_;
+ webrtc::AudioTransport* audio_callback_ RTC_GUARDED_BY(crit_);
- bool recording_; // True when audio is being pushed from the instance.
- bool playing_; // True when audio is being pulled by the instance.
+ bool recording_ RTC_GUARDED_BY(
+ crit_); // True when audio is being pushed from the instance.
+ bool playing_ RTC_GUARDED_BY(
+ crit_); // True when audio is being pulled by the instance.
bool play_is_initialized_; // True when the instance is ready to pull audio.
bool rec_is_initialized_; // True when the instance is ready to push audio.
@@ -197,13 +202,13 @@
// Input to and output from RecordedDataIsAvailable(..) makes it possible to
// modify the current mic level. The implementation does not care about the
// mic level so it just feeds back what it receives.
- uint32_t current_mic_level_;
+ uint32_t current_mic_level_ RTC_GUARDED_BY(crit_);
// next_frame_time_ is updated in a non-drifting manner to indicate the next
// wall clock time the next frame should be generated and received. started_
// ensures that next_frame_time_ can be initialized properly on first call.
- bool started_;
- int64_t next_frame_time_;
+ bool started_ RTC_GUARDED_BY(crit_);
+ int64_t next_frame_time_ RTC_GUARDED_BY(process_thread_checker_);
std::unique_ptr<rtc::Thread> process_thread_;
@@ -220,9 +225,7 @@
// Protects variables that are accessed from process_thread_ and
// the main thread.
rtc::CriticalSection crit_;
- // Protects |audio_callback_| that is accessed from process_thread_ and
- // the main thread.
- rtc::CriticalSection crit_callback_;
+ webrtc::SequenceChecker process_thread_checker_;
};
#endif // PC_TEST_FAKE_AUDIO_CAPTURE_MODULE_H_
