Improved timer implementation in AudioInputController

media/audio/audio_input_controller.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/audio/audio_input_controller.h"
 
 #include "base/bind.h"
 #include "base/threading/thread_restrictions.h"
 #include "media/base/limits.h"
 
 namespace {
 const int kMaxInputChannels = 2;
 const int kTimerResetInterval = 1;  // One second.
 }
 
 namespace media {
 
 // static
 AudioInputController::Factory* AudioInputController::factory_ = NULL;
 
 AudioInputController::AudioInputController(EventHandler* handler,
                                            SyncWriter* sync_writer)
     : 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() {
   DCHECK(kClosed == state_ || kCreated == state_ || kEmpty == state_);
 }
 
 // static
 scoped_refptr<AudioInputController> AudioInputController::Create(
     AudioManager* audio_manager,
     EventHandler* event_handler,
@@ skipping 54 matching lines @@
 }
 
 void AudioInputController::Record() {
   message_loop_->PostTask(FROM_HERE, base::Bind(
       &AudioInputController::DoRecord, this));
 }
 
 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);
 }
 
 void AudioInputController::SetVolume(double volume) {
   message_loop_->PostTask(FROM_HERE, base::Bind(
       &AudioInputController::DoSetVolume, this, volume));
 }
 
 void AudioInputController::SetAutomaticGainControl(bool enabled) {
@@ skipping 15 matching lines @@
   }
 
   if (stream_ && !stream_->Open()) {
     stream_->Close();
     stream_ = NULL;
     // TODO(satish): Define error types.
     handler_->OnError(this, 0);
     return;
   }
 
-  creator_loop_->PostTask(FROM_HERE, base::Bind(
-      &AudioInputController::DoResetNoDataTimer, this));
+  if (!no_data_timer_.get()) {

[tommi (sloooow) - chröme, 2012/04/17 13:12:49]

sorry, what I meant was this:

DCHECK(!no_data_timer_.get());
no_data_timer_.reset(...);

[henrika (OOO until Aug 14), 2012/04/17 13:38:31]

Done.

+    // 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);
 }
 
 void AudioInputController::DoRecord() {
   DCHECK(message_loop_->BelongsToCurrentThread());
 
   if (state_ != kCreated)
     return;
 
   {
     base::AutoLock auto_lock(lock_);
     state_ = kRecording;
   }
 
+  // Start the data timer. Once |kTimerResetInterval| seconds have passed,
+  // a callback to DoCheckForNoData() made.
+  no_data_timer_->Reset();
+
   stream_->Start(this);
   handler_->OnRecording(this);
 }
 
 void AudioInputController::DoClose() {
   DCHECK(message_loop_->BelongsToCurrentThread());
 
+  if (no_data_timer_.get()) {
+    // 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();
     }
 
     state_ = kClosed;
   }
 }
 
 void AudioInputController::DoReportError(int code) {
@@ skipping 28 matching lines @@
   DCHECK(message_loop_->BelongsToCurrentThread());
   DCHECK_NE(state_, kRecording);
 
   // Ensure that the AGC state only can be modified before streaming starts.
   if (state_ != kCreated || state_ == kRecording)
     return;
 
   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,
                                   uint32 size, uint32 hardware_delay_bytes,
                                   double volume) {
   {
     base::AutoLock auto_lock(lock_);
     if (state_ != kRecording)
       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()) {
     sync_writer_->Write(data, size, volume);
     sync_writer_->UpdateRecordedBytes(hardware_delay_bytes);
     return;
   }
 
   handler_->OnData(this, data, size);
 }
@@ skipping 20 matching lines @@
     stream_->Stop();
     stream_->Close();
     stream_ = NULL;
   }
 
   // Should be last in the method, do not touch "this" from here on.
   if (done != NULL)
     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