Removes flaky thread checker in AudioDeviceBuffer.

This CL removes a set of DCHECKs in AudioDeviceBuffer (ADB) where the goal has been
to ensure that some methods are called on one and the same native I/O thread.
The implementation of the ADB is platform independent but the underlying (driving)
audio components differ between platforms. This combination has shown to generate complex
corner cases such as:

- OS dependent I/O-thread(s) changes while audio is active
- OS dependent audio device changes and it leads to restart of native I/O threads
- Start/Stop of audio has different timing depending on platform and possibly also usage of
JNI and/or emulators.

To summarize: the gain of maintaining the current strict thread checking (in Debug mode)
is not worth all the efforts trying to resolve complex dynamic cases where the native
I/O threads changes ID.

TBR=glaznev

Bug: b/115385789
Change-Id: I681c89adec497a18b97d2a40421c04ea218fd919
Reviewed-on: https://webrtc-review.googlesource.com/100200
Commit-Queue: Henrik Andreassson <henrika@webrtc.org>
Reviewed-by: Henrik Andreassson <henrika@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24723}
This commit is contained in:
henrika
2018-09-13 13:01:14 +02:00
committed by Commit Bot
parent ef615ea7a3
commit 36b3179312
9 changed files with 8 additions and 61 deletions

View File

@ -219,7 +219,6 @@ void AAudioPlayer::HandleStreamDisconnected() {
}
// Perform a restart by first closing the disconnected stream and then start
// a new stream; this time using the new (preferred) audio output device.
audio_device_buffer_->NativeAudioPlayoutInterrupted();
StopPlayout();
InitPlayout();
StartPlayout();

View File

@ -212,7 +212,6 @@ void AAudioRecorder::HandleStreamDisconnected() {
// TODO(henrika): resolve issue where a one restart attempt leads to a long
// sequence of new calls to OnErrorCallback().
// See b/73148976 for details.
audio_device_buffer_->NativeAudioRecordingInterrupted();
StopRecording();
InitRecording();
StartRecording();

View File

@ -67,8 +67,6 @@ AudioDeviceBuffer::AudioDeviceBuffer()
RTC_LOG(WARNING) << "AUDIO_DEVICE_PLAYS_SINUS_TONE is defined!";
#endif
WebRtcSpl_Init();
playout_thread_checker_.DetachFromThread();
recording_thread_checker_.DetachFromThread();
}
AudioDeviceBuffer::~AudioDeviceBuffer() {
@ -99,7 +97,6 @@ void AudioDeviceBuffer::StartPlayout() {
return;
}
RTC_LOG(INFO) << __FUNCTION__;
playout_thread_checker_.DetachFromThread();
// Clear members tracking playout stats and do it on the task queue.
task_queue_.PostTask([this] { ResetPlayStats(); });
// Start a periodic timer based on task queue if not already done by the
@ -119,7 +116,6 @@ void AudioDeviceBuffer::StartRecording() {
return;
}
RTC_LOG(INFO) << __FUNCTION__;
recording_thread_checker_.DetachFromThread();
// Clear members tracking recording stats and do it on the task queue.
task_queue_.PostTask([this] { ResetRecStats(); });
// Start a periodic timer based on task queue if not already done by the
@ -222,30 +218,17 @@ size_t AudioDeviceBuffer::PlayoutChannels() const {
}
int32_t AudioDeviceBuffer::SetTypingStatus(bool typing_status) {
RTC_DCHECK_RUN_ON(&recording_thread_checker_);
typing_status_ = typing_status;
return 0;
}
void AudioDeviceBuffer::NativeAudioPlayoutInterrupted() {
RTC_DCHECK(main_thread_checker_.CalledOnValidThread());
playout_thread_checker_.DetachFromThread();
}
void AudioDeviceBuffer::NativeAudioRecordingInterrupted() {
RTC_DCHECK(main_thread_checker_.CalledOnValidThread());
recording_thread_checker_.DetachFromThread();
}
void AudioDeviceBuffer::SetVQEData(int play_delay_ms, int rec_delay_ms) {
RTC_DCHECK_RUN_ON(&recording_thread_checker_);
play_delay_ms_ = play_delay_ms;
rec_delay_ms_ = rec_delay_ms;
}
int32_t AudioDeviceBuffer::SetRecordedBuffer(const void* audio_buffer,
size_t samples_per_channel) {
RTC_DCHECK_RUN_ON(&recording_thread_checker_);
// Copy the complete input buffer to the local buffer.
const size_t old_size = rec_buffer_.size();
rec_buffer_.SetData(static_cast<const int16_t*>(audio_buffer),
@ -277,7 +260,6 @@ int32_t AudioDeviceBuffer::SetRecordedBuffer(const void* audio_buffer,
}
int32_t AudioDeviceBuffer::DeliverRecordedData() {
RTC_DCHECK_RUN_ON(&recording_thread_checker_);
if (!audio_transport_cb_) {
RTC_LOG(LS_WARNING) << "Invalid audio transport";
return 0;
@ -297,7 +279,6 @@ int32_t AudioDeviceBuffer::DeliverRecordedData() {
}
int32_t AudioDeviceBuffer::RequestPlayoutData(size_t samples_per_channel) {
RTC_DCHECK_RUN_ON(&playout_thread_checker_);
// The consumer can change the requested size on the fly and we therefore
// resize the buffer accordingly. Also takes place at the first call to this
// method.
@ -342,7 +323,6 @@ int32_t AudioDeviceBuffer::RequestPlayoutData(size_t samples_per_channel) {
}
int32_t AudioDeviceBuffer::GetPlayoutData(void* audio_buffer) {
RTC_DCHECK_RUN_ON(&playout_thread_checker_);
RTC_DCHECK_GT(play_buffer_.size(), 0);
#ifdef AUDIO_DEVICE_PLAYS_SINUS_TONE
const double phase_increment =
@ -484,7 +464,6 @@ void AudioDeviceBuffer::ResetPlayStats() {
void AudioDeviceBuffer::UpdateRecStats(int16_t max_abs,
size_t samples_per_channel) {
RTC_DCHECK_RUN_ON(&recording_thread_checker_);
rtc::CritScope cs(&lock_);
++stats_.rec_callbacks;
stats_.rec_samples += samples_per_channel;
@ -495,7 +474,6 @@ void AudioDeviceBuffer::UpdateRecStats(int16_t max_abs,
void AudioDeviceBuffer::UpdatePlayStats(int16_t max_abs,
size_t samples_per_channel) {
RTC_DCHECK_RUN_ON(&playout_thread_checker_);
rtc::CritScope cs(&lock_);
++stats_.play_callbacks;
stats_.play_samples += samples_per_channel;

View File

@ -104,13 +104,6 @@ class AudioDeviceBuffer {
int32_t SetTypingStatus(bool typing_status);
// Called on iOS and Android where the native audio layer can be interrupted
// by other audio applications. These methods can then be used to reset
// internal states and detach thread checkers to allow for new audio sessions
// where native callbacks may come from a new set of I/O threads.
void NativeAudioPlayoutInterrupted();
void NativeAudioRecordingInterrupted();
private:
// Starts/stops periodic logging of audio stats.
void StartPeriodicLogging();
@ -145,12 +138,6 @@ class AudioDeviceBuffer {
// Main thread on which this object is created.
rtc::ThreadChecker main_thread_checker_;
// Native (platform specific) audio thread driving the playout side.
rtc::ThreadChecker playout_thread_checker_;
// Native (platform specific) audio thread driving the recording side.
rtc::ThreadChecker recording_thread_checker_;
rtc::CriticalSection lock_;
// Task queue used to invoke LogStats() periodically. Tasks are executed on a
@ -183,18 +170,18 @@ class AudioDeviceBuffer {
// Buffer used for audio samples to be played out. Size can be changed
// dynamically. The 16-bit samples are interleaved, hence the size is
// proportional to the number of channels.
rtc::BufferT<int16_t> play_buffer_ RTC_GUARDED_BY(playout_thread_checker_);
rtc::BufferT<int16_t> play_buffer_;
// Byte buffer used for recorded audio samples. Size can be changed
// dynamically.
rtc::BufferT<int16_t> rec_buffer_ RTC_GUARDED_BY(recording_thread_checker_);
rtc::BufferT<int16_t> rec_buffer_;
// Contains true of a key-press has been detected.
bool typing_status_ RTC_GUARDED_BY(recording_thread_checker_);
bool typing_status_;
// Delay values used by the AEC.
int play_delay_ms_ RTC_GUARDED_BY(recording_thread_checker_);
int rec_delay_ms_ RTC_GUARDED_BY(recording_thread_checker_);
int play_delay_ms_;
int rec_delay_ms_;
// Counts number of times LogStats() has been called.
size_t num_stat_reports_ RTC_GUARDED_BY(task_queue_);
@ -204,8 +191,8 @@ class AudioDeviceBuffer {
// Counts number of audio callbacks modulo 50 to create a signal when
// a new storage of audio stats shall be done.
int16_t rec_stat_count_ RTC_GUARDED_BY(recording_thread_checker_);
int16_t play_stat_count_ RTC_GUARDED_BY(playout_thread_checker_);
int16_t rec_stat_count_;
int16_t play_stat_count_;
// Time stamps of when playout and recording starts.
int64_t play_start_time_ RTC_GUARDED_BY(main_thread_checker_);
@ -231,7 +218,7 @@ class AudioDeviceBuffer {
// Should *never* be defined in production builds. Only used for testing.
// When defined, the output signal will be replaced by a sinus tone at 440Hz.
#ifdef AUDIO_DEVICE_PLAYS_SINUS_TONE
double phase_ RTC_GUARDED_BY(playout_thread_checker_);
double phase_;
#endif
};

View File

@ -903,12 +903,6 @@ void AudioDeviceIOS::PrepareForNewStart() {
// which means that we must detach thread checkers here to be prepared for an
// upcoming new audio stream.
io_thread_checker_.DetachFromThread();
// The audio device buffer must also be informed about the interrupted
// state so it can detach its thread checkers as well.
if (audio_device_buffer_) {
audio_device_buffer_->NativeAudioPlayoutInterrupted();
audio_device_buffer_->NativeAudioRecordingInterrupted();
}
}
} // namespace webrtc

View File

@ -582,8 +582,6 @@ bool CoreAudioBase::Stop() {
// error callbacks.
if (!IsRestarting()) {
thread_checker_audio_.DetachFromThread();
IsOutput() ? audio_device_buffer_->NativeAudioPlayoutInterrupted()
: audio_device_buffer_->NativeAudioRecordingInterrupted();
}
// Release all allocated COM interfaces to allow for a restart without

View File

@ -235,7 +235,6 @@ void AAudioPlayer::HandleStreamDisconnected() {
}
// Perform a restart by first closing the disconnected stream and then start
// a new stream; this time using the new (preferred) audio output device.
audio_device_buffer_->NativeAudioPlayoutInterrupted();
StopPlayout();
InitPlayout();
StartPlayout();

View File

@ -224,7 +224,6 @@ void AAudioRecorder::HandleStreamDisconnected() {
// TODO(henrika): resolve issue where a one restart attempt leads to a long
// sequence of new calls to OnErrorCallback().
// See b/73148976 for details.
audio_device_buffer_->NativeAudioRecordingInterrupted();
StopRecording();
InitRecording();
StartRecording();

View File

@ -915,12 +915,6 @@ void AudioDeviceIOS::PrepareForNewStart() {
// which means that we must detach thread checkers here to be prepared for an
// upcoming new audio stream.
io_thread_checker_.DetachFromThread();
// The audio device buffer must also be informed about the interrupted
// state so it can detach its thread checkers as well.
if (audio_device_buffer_) {
audio_device_buffer_->NativeAudioPlayoutInterrupted();
audio_device_buffer_->NativeAudioRecordingInterrupted();
}
}
bool AudioDeviceIOS::IsInterrupted() {