Revert of Adds data logging in native AudioDeviceBuffer class (patchset #10 id:180001 of https://codereview.webrtc.org/2132613002/ )
Reason for revert:
Seems to break things upstream.
Original issue's description:
> Adds data logging in native AudioDeviceBuffer class.
>
> Goal is to provide periodic logging of most essential audio parameters
> for playout and recording sides. It will allow us to track if the native audio layer is working as intended.
>
> BUG=NONE
>
> Committed: https://crrev.com/348e411dd27e6dbe9b84b27ce46e9b7c657c1eae
> Cr-Commit-Position: refs/heads/master@{#13440}
TBR=stefan@webrtc.org,henrika@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=NONE
Review-Url: https://codereview.webrtc.org/2139233002
Cr-Commit-Position: refs/heads/master@{#13441}
diff --git a/webrtc/modules/audio_device/audio_device_buffer.cc b/webrtc/modules/audio_device/audio_device_buffer.cc
index b40d5af..fb82b91 100644
--- a/webrtc/modules/audio_device/audio_device_buffer.cc
+++ b/webrtc/modules/audio_device/audio_device_buffer.cc
@@ -10,29 +10,21 @@
#include "webrtc/modules/audio_device/audio_device_buffer.h"
-#include "webrtc/base/bind.h"
#include "webrtc/base/checks.h"
#include "webrtc/base/logging.h"
#include "webrtc/base/format_macros.h"
-#include "webrtc/base/timeutils.h"
#include "webrtc/modules/audio_device/audio_device_config.h"
+#include "webrtc/system_wrappers/include/critical_section_wrapper.h"
namespace webrtc {
static const int kHighDelayThresholdMs = 300;
static const int kLogHighDelayIntervalFrames = 500; // 5 seconds.
-static const char kTimerQueueName[] = "AudioDeviceBufferTimer";
-
-// Time between two sucessive calls to LogStats().
-static const size_t kTimerIntervalInSeconds = 10;
-static const size_t kTimerIntervalInMilliseconds =
- kTimerIntervalInSeconds * rtc::kNumMillisecsPerSec;
-
AudioDeviceBuffer::AudioDeviceBuffer()
- : _ptrCbAudioTransport(nullptr),
- task_queue_(kTimerQueueName),
- timer_has_started_(false),
+ : _critSect(*CriticalSectionWrapper::CreateCriticalSection()),
+ _critSectCb(*CriticalSectionWrapper::CreateCriticalSection()),
+ _ptrCbAudioTransport(nullptr),
_recSampleRate(0),
_playSampleRate(0),
_recChannels(0),
@@ -53,72 +45,58 @@
_recDelayMS(0),
_clockDrift(0),
// Set to the interval in order to log on the first occurrence.
- high_delay_counter_(kLogHighDelayIntervalFrames),
- num_stat_reports_(0),
- rec_callbacks_(0),
- last_rec_callbacks_(0),
- play_callbacks_(0),
- last_play_callbacks_(0),
- rec_samples_(0),
- last_rec_samples_(0),
- play_samples_(0),
- last_play_samples_(0),
- last_log_stat_time_(0) {
+ high_delay_counter_(kLogHighDelayIntervalFrames) {
LOG(INFO) << "AudioDeviceBuffer::ctor";
memset(_recBuffer, 0, kMaxBufferSizeBytes);
memset(_playBuffer, 0, kMaxBufferSizeBytes);
}
AudioDeviceBuffer::~AudioDeviceBuffer() {
- RTC_DCHECK(thread_checker_.CalledOnValidThread());
LOG(INFO) << "AudioDeviceBuffer::~dtor";
- _recFile.Flush();
- _recFile.CloseFile();
- delete &_recFile;
+ {
+ CriticalSectionScoped lock(&_critSect);
- _playFile.Flush();
- _playFile.CloseFile();
- delete &_playFile;
+ _recFile.Flush();
+ _recFile.CloseFile();
+ delete &_recFile;
+
+ _playFile.Flush();
+ _playFile.CloseFile();
+ delete &_playFile;
+ }
+
+ delete &_critSect;
+ delete &_critSectCb;
}
int32_t AudioDeviceBuffer::RegisterAudioCallback(
AudioTransport* audioCallback) {
LOG(INFO) << __FUNCTION__;
- rtc::CritScope lock(&_critSectCb);
+ CriticalSectionScoped lock(&_critSectCb);
_ptrCbAudioTransport = audioCallback;
return 0;
}
int32_t AudioDeviceBuffer::InitPlayout() {
- RTC_DCHECK(thread_checker_.CalledOnValidThread());
LOG(INFO) << __FUNCTION__;
- if (!timer_has_started_) {
- StartTimer();
- timer_has_started_ = true;
- }
return 0;
}
int32_t AudioDeviceBuffer::InitRecording() {
- RTC_DCHECK(thread_checker_.CalledOnValidThread());
LOG(INFO) << __FUNCTION__;
- if (!timer_has_started_) {
- StartTimer();
- timer_has_started_ = true;
- }
return 0;
}
int32_t AudioDeviceBuffer::SetRecordingSampleRate(uint32_t fsHz) {
LOG(INFO) << "SetRecordingSampleRate(" << fsHz << ")";
- rtc::CritScope lock(&_critSect);
+ CriticalSectionScoped lock(&_critSect);
_recSampleRate = fsHz;
return 0;
}
int32_t AudioDeviceBuffer::SetPlayoutSampleRate(uint32_t fsHz) {
LOG(INFO) << "SetPlayoutSampleRate(" << fsHz << ")";
- rtc::CritScope lock(&_critSect);
+ CriticalSectionScoped lock(&_critSect);
_playSampleRate = fsHz;
return 0;
}
@@ -132,7 +110,7 @@
}
int32_t AudioDeviceBuffer::SetRecordingChannels(size_t channels) {
- rtc::CritScope lock(&_critSect);
+ CriticalSectionScoped lock(&_critSect);
_recChannels = channels;
_recBytesPerSample =
2 * channels; // 16 bits per sample in mono, 32 bits in stereo
@@ -140,7 +118,7 @@
}
int32_t AudioDeviceBuffer::SetPlayoutChannels(size_t channels) {
- rtc::CritScope lock(&_critSect);
+ CriticalSectionScoped lock(&_critSect);
_playChannels = channels;
// 16 bits per sample in mono, 32 bits in stereo
_playBytesPerSample = 2 * channels;
@@ -149,7 +127,7 @@
int32_t AudioDeviceBuffer::SetRecordingChannel(
const AudioDeviceModule::ChannelType channel) {
- rtc::CritScope lock(&_critSect);
+ CriticalSectionScoped lock(&_critSect);
if (_recChannels == 1) {
return -1;
@@ -215,7 +193,7 @@
int32_t AudioDeviceBuffer::StartInputFileRecording(
const char fileName[kAdmMaxFileNameSize]) {
- rtc::CritScope lock(&_critSect);
+ CriticalSectionScoped lock(&_critSect);
_recFile.Flush();
_recFile.CloseFile();
@@ -224,7 +202,7 @@
}
int32_t AudioDeviceBuffer::StopInputFileRecording() {
- rtc::CritScope lock(&_critSect);
+ CriticalSectionScoped lock(&_critSect);
_recFile.Flush();
_recFile.CloseFile();
@@ -234,7 +212,7 @@
int32_t AudioDeviceBuffer::StartOutputFileRecording(
const char fileName[kAdmMaxFileNameSize]) {
- rtc::CritScope lock(&_critSect);
+ CriticalSectionScoped lock(&_critSect);
_playFile.Flush();
_playFile.CloseFile();
@@ -243,7 +221,7 @@
}
int32_t AudioDeviceBuffer::StopOutputFileRecording() {
- rtc::CritScope lock(&_critSect);
+ CriticalSectionScoped lock(&_critSect);
_playFile.Flush();
_playFile.CloseFile();
@@ -253,7 +231,7 @@
int32_t AudioDeviceBuffer::SetRecordedBuffer(const void* audioBuffer,
size_t nSamples) {
- rtc::CritScope lock(&_critSect);
+ CriticalSectionScoped lock(&_critSect);
if (_recBytesPerSample == 0) {
assert(false);
@@ -292,16 +270,11 @@
_recFile.Write(&_recBuffer[0], _recSize);
}
- // Update some stats but do it on the task queue to ensure that the members
- // are modified and read on the same thread.
- task_queue_.PostTask(
- rtc::Bind(&AudioDeviceBuffer::UpdateRecStats, this, nSamples));
-
return 0;
}
int32_t AudioDeviceBuffer::DeliverRecordedData() {
- rtc::CritScope lock(&_critSectCb);
+ CriticalSectionScoped lock(&_critSectCb);
// Ensure that user has initialized all essential members
if ((_recSampleRate == 0) || (_recSamples == 0) ||
(_recBytesPerSample == 0) || (_recChannels == 0)) {
@@ -336,7 +309,7 @@
// TOOD(henrika): improve bad locking model and make it more clear that only
// 10ms buffer sizes is supported in WebRTC.
{
- rtc::CritScope lock(&_critSect);
+ CriticalSectionScoped lock(&_critSect);
// Store copies under lock and use copies hereafter to avoid race with
// setter methods.
@@ -359,7 +332,7 @@
size_t nSamplesOut(0);
- rtc::CritScope lock(&_critSectCb);
+ CriticalSectionScoped lock(&_critSectCb);
// It is currently supported to start playout without a valid audio
// transport object. Leads to warning and silence.
@@ -378,16 +351,11 @@
LOG(LS_ERROR) << "NeedMorePlayData() failed";
}
- // Update some stats but do it on the task queue to ensure that access of
- // members is serialized hence avoiding usage of locks.
- task_queue_.PostTask(
- rtc::Bind(&AudioDeviceBuffer::UpdatePlayStats, this, nSamplesOut));
-
return static_cast<int32_t>(nSamplesOut);
}
int32_t AudioDeviceBuffer::GetPlayoutData(void* audioBuffer) {
- rtc::CritScope lock(&_critSect);
+ CriticalSectionScoped lock(&_critSect);
RTC_CHECK_LE(_playSize, kMaxBufferSizeBytes);
memcpy(audioBuffer, &_playBuffer[0], _playSize);
@@ -400,67 +368,4 @@
return static_cast<int32_t>(_playSamples);
}
-void AudioDeviceBuffer::StartTimer() {
- last_log_stat_time_ = rtc::TimeMillis();
- task_queue_.PostDelayedTask(rtc::Bind(&AudioDeviceBuffer::LogStats, this),
- kTimerIntervalInMilliseconds);
-}
-
-void AudioDeviceBuffer::LogStats() {
- RTC_DCHECK(task_queue_.IsCurrent());
-
- int64_t now_time = rtc::TimeMillis();
- int64_t next_callback_time = now_time + kTimerIntervalInMilliseconds;
- int64_t time_since_last = rtc::TimeDiff(now_time, last_log_stat_time_);
- last_log_stat_time_ = now_time;
-
- // Log the latest statistics but skip the first 10 seconds since we are not
- // sure of the exact starting point. I.e., the first log printout will be
- // after ~20 seconds.
- if (++num_stat_reports_ > 1) {
- uint32_t diff_samples = rec_samples_ - last_rec_samples_;
- uint32_t rate = diff_samples / kTimerIntervalInSeconds;
- LOG(INFO) << "[REC : " << time_since_last << "msec, "
- << _recSampleRate / 1000
- << "kHz] callbacks: " << rec_callbacks_ - last_rec_callbacks_
- << ", "
- << "samples: " << diff_samples << ", "
- << "rate: " << rate;
-
- diff_samples = play_samples_ - last_play_samples_;
- rate = diff_samples / kTimerIntervalInSeconds;
- LOG(INFO) << "[PLAY: " << time_since_last << "msec, "
- << _playSampleRate / 1000
- << "kHz] callbacks: " << play_callbacks_ - last_play_callbacks_
- << ", "
- << "samples: " << diff_samples << ", "
- << "rate: " << rate;
- }
-
- last_rec_callbacks_ = rec_callbacks_;
- last_play_callbacks_ = play_callbacks_;
- last_rec_samples_ = rec_samples_;
- last_play_samples_ = play_samples_;
-
- int64_t time_to_wait_ms = next_callback_time - rtc::TimeMillis();
- RTC_DCHECK_GT(time_to_wait_ms, 0) << "Invalid timer interval";
-
- // Update some stats but do it on the task queue to ensure that access of
- // members is serialized hence avoiding usage of locks.
- task_queue_.PostDelayedTask(rtc::Bind(&AudioDeviceBuffer::LogStats, this),
- time_to_wait_ms);
-}
-
-void AudioDeviceBuffer::UpdateRecStats(size_t num_samples) {
- RTC_DCHECK(task_queue_.IsCurrent());
- ++rec_callbacks_;
- rec_samples_ += num_samples;
-}
-
-void AudioDeviceBuffer::UpdatePlayStats(size_t num_samples) {
- RTC_DCHECK(task_queue_.IsCurrent());
- ++play_callbacks_;
- play_samples_ += num_samples;
-}
-
} // namespace webrtc
diff --git a/webrtc/modules/audio_device/audio_device_buffer.h b/webrtc/modules/audio_device/audio_device_buffer.h
index ee6b229..1267e08 100644
--- a/webrtc/modules/audio_device/audio_device_buffer.h
+++ b/webrtc/modules/audio_device/audio_device_buffer.h
@@ -8,12 +8,9 @@
* be found in the AUTHORS file in the root of the source tree.
*/
-#ifndef WEBRTC_MODULES_AUDIO_DEVICE_AUDIO_DEVICE_BUFFER_H_
-#define WEBRTC_MODULES_AUDIO_DEVICE_AUDIO_DEVICE_BUFFER_H_
+#ifndef WEBRTC_AUDIO_DEVICE_AUDIO_DEVICE_BUFFER_H
+#define WEBRTC_AUDIO_DEVICE_AUDIO_DEVICE_BUFFER_H
-#include "webrtc/base/criticalsection.h"
-#include "webrtc/base/task_queue.h"
-#include "webrtc/base/thread_checker.h"
#include "webrtc/modules/audio_device/include/audio_device.h"
#include "webrtc/system_wrappers/include/file_wrapper.h"
#include "webrtc/typedefs.h"
@@ -66,36 +63,11 @@
int32_t SetTypingStatus(bool typingStatus);
private:
- // Posts the first delayed task in the task queue and starts the periodic
- // timer.
- void StartTimer();
-
- // Called periodically on the internal thread created by the TaskQueue.
- void LogStats();
-
- // Updates counters in each play/record callback but does it on the task
- // queue to ensure that they can be read by LogStats() without any locks since
- // each task is serialized by the task queue.
- void UpdateRecStats(size_t num_samples);
- void UpdatePlayStats(size_t num_samples);
-
- // Ensures that methods are called on the same thread as the thread that
- // creates this object.
- rtc::ThreadChecker thread_checker_;
-
- rtc::CriticalSection _critSect;
- rtc::CriticalSection _critSectCb;
+ CriticalSectionWrapper& _critSect;
+ CriticalSectionWrapper& _critSectCb;
AudioTransport* _ptrCbAudioTransport;
- // Task queue used to invoke LogStats() periodically. Tasks are executed on a
- // worker thread but it does not necessarily have to be the same thread for
- // each task.
- rtc::TaskQueue task_queue_;
-
- // Ensures that the timer is only started once.
- bool timer_has_started_;
-
uint32_t _recSampleRate;
uint32_t _playSampleRate;
@@ -135,40 +107,8 @@
int _recDelayMS;
int _clockDrift;
int high_delay_counter_;
-
- // Counts number of times LogStats() has been called.
- size_t num_stat_reports_;
-
- // Total number of recording callbacks where the source provides 10ms audio
- // data each time.
- uint64_t rec_callbacks_;
-
- // Total number of recording callbacks stored at the last timer task.
- uint64_t last_rec_callbacks_;
-
- // Total number of playback callbacks where the sink asks for 10ms audio
- // data each time.
- uint64_t play_callbacks_;
-
- // Total number of playout callbacks stored at the last timer task.
- uint64_t last_play_callbacks_;
-
- // Total number of recorded audio samples.
- uint64_t rec_samples_;
-
- // Total number of recorded samples stored at the previous timer task.
- uint64_t last_rec_samples_;
-
- // Total number of played audio samples.
- uint64_t play_samples_;
-
- // Total number of played samples stored at the previous timer task.
- uint64_t last_play_samples_;
-
- // Time stamp of last stat report.
- uint64_t last_log_stat_time_;
};
} // namespace webrtc
-#endif // WEBRTC_MODULES_AUDIO_DEVICE_AUDIO_DEVICE_BUFFER_H_
+#endif // WEBRTC_AUDIO_DEVICE_AUDIO_DEVICE_BUFFER_H
