Skip to content

GitLab

T
tangled
Commit d4049571 authored by henrika@chromium.org's avatar henrika@chromium.org
Browse files
Options

Improved timer implementation in AudioInputController since... 

Improved timer implementation in AudioInputController since AudioInputController::DoResetNoDataTimer() has an extreme task execution count in chrome://profiler.

There is no longer a PostTask in OnData() to reset the data timer. Instead, a simple flag is set and this flag is checked, and reset, on a periodic basis. If packets no longer are generated, the cleared flag will be detected an and OnError() callback will be given to the event handler.

I have verified the new design in different ways:

1) Verified that no AudioInputController function pops up in the profiler.
2) media_unittest --gtest_filter=AudioInputControllerTest*RecordAndError
3) Tried out the speech input API and the removed the microphone while recording and verified that it worked.
4) Same as 3) but for WebRTC in loopback using the https://apprtc.appspot.com/?debug=loopback demo.

BUG=123588
TEST=media_unittest --gtest_filter=AudioInputControllerTest*RecordAndError

Review URL: http://codereview.chromium.org/9956169

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@132583 0039d316-1c4b-4281-b951-d872f2087c98
parent 285f4bb8
Hide whitespace changes
Inline Side-by-side
Showing with 73 additions and 40 deletions
media/audio/audio_input_controller.cc
View file @ d4049571
......@@ -23,14 +23,11 @@ AudioInputController::AudioInputController(EventHandler* handler,
: creator_loop_(base::MessageLoopProxy::current()),
handler_(handler),
stream_(NULL),
data_is_active_(false),
state_(kEmpty),
sync_writer_(sync_writer),
max_volume_(0.0) {
DCHECK(creator_loop_);
no_data_timer_.reset(new base::DelayTimer<AudioInputController>(FROM_HERE,
base::TimeDelta::FromSeconds(kTimerResetInterval),
this,
&AudioInputController::DoReportNoDataError));
}
AudioInputController::~AudioInputController() {
......@@ -105,10 +102,7 @@ void AudioInputController::Record() {
void AudioInputController::Close(const base::Closure& closed_task) {
DCHECK(!closed_task.is_null());
DCHECK(creator_loop_->BelongsToCurrentThread());
// See crbug.com/119783: Deleting the timer now to avoid disaster if
// AudioInputController is destructed on a thread other than the creator
// thread.
no_data_timer_.reset();
message_loop_->PostTaskAndReply(
FROM_HERE, base::Bind(&AudioInputController::DoClose, this), closed_task);
}
......@@ -144,8 +138,14 @@ void AudioInputController::DoCreate(AudioManager* audio_manager,
return;
}
creator_loop_->PostTask(FROM_HERE, base::Bind(
&AudioInputController::DoResetNoDataTimer, this));
DCHECK(!no_data_timer_.get());
// Create the data timer which will call DoCheckForNoData() after a delay
// of |kTimerResetInterval| seconds. The timer is started in DoRecord()
// and restarted in each DoCheckForNoData() callback.
no_data_timer_.reset(new base::DelayTimer<AudioInputController>(FROM_HERE,
base::TimeDelta::FromSeconds(kTimerResetInterval),
this,
&AudioInputController::DoCheckForNoData));
state_ = kCreated;
handler_->OnCreated(this);
......@@ -162,6 +162,10 @@ void AudioInputController::DoRecord() {
state_ = kRecording;
}
// Start the data timer. Once |kTimerResetInterval| seconds have passed,
// a callback to DoCheckForNoData() is made.
no_data_timer_->Reset();
stream_->Start(this);
handler_->OnRecording(this);
}
......@@ -169,8 +173,12 @@ void AudioInputController::DoRecord() {
void AudioInputController::DoClose() {
DCHECK(message_loop_->BelongsToCurrentThread());
// Delete the timer on the same thread that created it.
no_data_timer_.reset();
if (state_ != kClosed) {
DoStopCloseAndClearStream(NULL);
SetDataIsActive(false);
if (LowLatencyMode()) {
sync_writer_->Close();
......@@ -219,19 +227,24 @@ void AudioInputController::DoSetAutomaticGainControl(bool enabled) {
stream_->SetAutomaticGainControl(enabled);
}
void AudioInputController::DoReportNoDataError() {
DCHECK(creator_loop_->BelongsToCurrentThread());
void AudioInputController::DoCheckForNoData() {
DCHECK(message_loop_->BelongsToCurrentThread());
// Error notifications should be sent on the audio-manager thread.
int code = 0;
message_loop_->PostTask(FROM_HERE, base::Bind(
&AudioInputController::DoReportError, this, code));
}
if (!GetDataIsActive()) {
// The data-is-active marker will be false only if it has been more than
// one second since a data packet was recorded. This can happen if a
// capture device has been removed or disabled.
handler_->OnError(this, 0);
return;
}
void AudioInputController::DoResetNoDataTimer() {
DCHECK(creator_loop_->BelongsToCurrentThread());
if (no_data_timer_.get())
no_data_timer_->Reset();
// Mark data as non-active. The flag will be re-enabled in OnData() each
// time a data packet is received. Hence, under normal conditions, the
// flag will only be disabled during a very short period.
SetDataIsActive(false);
// Restart the timer to ensure that we check the flag in one second again.
no_data_timer_->Reset();
}
void AudioInputController::OnData(AudioInputStream* stream, const uint8* data,
......@@ -243,8 +256,9 @@ void AudioInputController::OnData(AudioInputStream* stream, const uint8* data,
return;
}
creator_loop_->PostTask(FROM_HERE, base::Bind(
&AudioInputController::DoResetNoDataTimer, this));
// Mark data as active to ensure that the periodic calls to
// DoCheckForNoData() does not report an error to the event handler.
SetDataIsActive(true);
// Use SyncSocket if we are in a low-latency mode.
if (LowLatencyMode()) {
......@@ -285,4 +299,12 @@ void AudioInputController::DoStopCloseAndClearStream(
done->Signal();
}
void AudioInputController::SetDataIsActive(bool enabled) {
base::subtle::Release_Store(&data_is_active_, enabled);
}
bool AudioInputController::GetDataIsActive() {
return (base::subtle::Acquire_Load(&data_is_active_) != false);
}
} // namespace media
media/audio/audio_input_controller.h
View file @ d4049571
......@@ -6,6 +6,7 @@
#define MEDIA_AUDIO_AUDIO_INPUT_CONTROLLER_H_
#include <string>
#include "base/atomicops.h"
#include "base/callback.h"
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
......@@ -42,24 +43,26 @@
// User AudioInputController EventHandler
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// CrateLowLatency() ==> DoCreate()
// AudioManager::MakeAudioInputStream()
// AudioInputStream::Open()
// .- - - - - - - - - - - - -> OnError()
// DoResetNoDataTimer (posted on creating tread)
// AudioManager::MakeAudioInputStream()
// AudioInputStream::Open()
// .- - - - - - - - - - - - -> OnError()
// create the data timer
// .-------------------------> OnCreated()
// kCreated
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// Record() ==> DoRecord()
// AudioInputStream::Start()
// AudioInputStream::Start()
// .-------------------------> OnRecording()
// start the data timer
// kRecording
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// Close() ==> DoClose()
// state_ = kClosed
// AudioInputStream::Stop()
// AudioInputStream::Close()
// SyncWriter::Close()
// Closure::Run() <--------------.
// delete the data timer
// state_ = kClosed
// AudioInputStream::Stop()
// AudioInputStream::Close()
// SyncWriter::Close()
// Closure::Run() <-----------------.
// (closure-task)
//
// The audio thread itself is owned by the AudioManager that the
......@@ -198,15 +201,17 @@ class MEDIA_EXPORT AudioInputController
void DoSetVolume(double volume);
void DoSetAutomaticGainControl(bool enabled);
// Methods which ensures that OnError() is triggered when data recording
// times out. Both are called on the creating thread.
void DoReportNoDataError();
void DoResetNoDataTimer();
// Method which ensures that OnError() is triggered when data recording
// times out. Called on the audio thread.
void DoCheckForNoData();
// Helper method that stops, closes, and NULL:s |*stream_|.
// Signals event when done if the event is not NULL.
void DoStopCloseAndClearStream(base::WaitableEvent* done);
void SetDataIsActive(bool enabled);
bool GetDataIsActive();
// Gives access to the message loop of the creating thread.
scoped_refptr<base::MessageLoopProxy> creator_loop_;
......@@ -220,13 +225,19 @@ class MEDIA_EXPORT AudioInputController
// Pointer to the audio input stream object.
AudioInputStream* stream_;
// |no_data_timer_| is used to call DoReportNoDataError() when we stop
// receiving OnData() calls without an OnClose() call. This can occur
// |no_data_timer_| is used to call OnError() when we stop receiving
// OnData() calls without an OnClose() call. This can occur
// when an audio input device is unplugged whilst recording on Windows.
// See http://crbug.com/79936 for details.
// This member is only touched by the creating thread.
// This member is only touched by the audio thread.
scoped_ptr<base::DelayTimer<AudioInputController> > no_data_timer_;
// This flag is used to signal that we are receiving OnData() calls, i.e,
// that data is active. It can be touched by the audio thread and by the
// low-level audio thread which calls OnData(). E.g. on Windows, the
// low-level audio thread is called wasapi_capture_thread.
base::subtle::Atomic32 data_is_active_;
// |state_| is written on the audio thread and is read on the hardware audio
// thread. These operations need to be locked. But lock is not required for
// reading on the audio input controller thread.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment