admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

FakeAudioCaptureModule: remove lock recursions.

Browse Source 
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}
This commit is contained in:
Markus Handell 2020-05-18 17:35:44 +02:00 committed by Commit Bot
parent 8de900cdcb
commit 2af35ab984
3 changed files with 66 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
pc/BUILD.gn
View File

@ -499,6 +499,7 @@ if (rtc_include_tests) {
"../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",

72
pc/test/fake_audio_capture_module.cc
View File

@ -47,7 +47,9 @@ FakeAudioCaptureModule::FakeAudioCaptureModule()
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::ActiveAudioLayer(
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 @@ void FakeAudioCaptureModule::UpdateProcessing(bool start) {
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::ProcessFrameP() {
}
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 @@ void FakeAudioCaptureModule::ReceiveFrameP() {
// 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(&current_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;
}

53
pc/test/fake_audio_capture_module.h
View File

@ -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 @@ class FakeAudioCaptureModule : public webrtc::AudioDeviceModule,
// 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 @@ class FakeAudioCaptureModule : public webrtc::AudioDeviceModule,
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 @@ class FakeAudioCaptureModule : public webrtc::AudioDeviceModule,
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 @@ class FakeAudioCaptureModule : public webrtc::AudioDeviceModule,
// 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 @@ class FakeAudioCaptureModule : public webrtc::AudioDeviceModule,
// 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 @@ class FakeAudioCaptureModule : public webrtc::AudioDeviceModule,
// 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_