Extended sanity checks in WASAPIAudioOutputStream

media/audio/win/audio_low_latency_output_win.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/win/audio_low_latency_output_win.h"
 
 #include <Functiondiscoverykeys_devpkey.h>
 
 #include "base/command_line.h"
 #include "base/logging.h"
@@ skipping 371 matching lines @@
     return AUDCLNT_SHAREMODE_EXCLUSIVE;
   return AUDCLNT_SHAREMODE_SHARED;
 }
 
 WASAPIAudioOutputStream::WASAPIAudioOutputStream(AudioManagerWin* manager,
                                                  const AudioParameters& params,
                                                  ERole device_role)
     : creating_thread_id_(base::PlatformThread::CurrentId()),
       manager_(manager),
       opened_(false),
-      started_(false),
       restart_rendering_mode_(false),
       volume_(1.0),
       endpoint_buffer_size_frames_(0),
       device_role_(device_role),
       share_mode_(GetShareMode()),
       client_channel_count_(params.channels()),
       num_written_frames_(0),
       source_(NULL),
       audio_bus_(AudioBus::Create(params)) {
   DCHECK(manager_);
@@ skipping 152 matching lines @@
   hr = device_enumerator_->RegisterEndpointNotificationCallback(this);
   if (FAILED(hr))
     RecordFallbackStats();
 
   opened_ = true;
   return SUCCEEDED(hr);
 }
 
 void WASAPIAudioOutputStream::Start(AudioSourceCallback* callback) {
   DCHECK_EQ(GetCurrentThreadId(), creating_thread_id_);
-  DCHECK(callback);
-  DLOG_IF(ERROR, !opened_) << "Open() has not been called successfully";
-  if (!opened_)
+  CHECK(callback);
+  CHECK(opened_);
+
+  if (render_thread_.get()) {
+    CHECK_EQ(callback, source_);
     return;
-
-  if (started_)
-    return;
+  }
 
   if (restart_rendering_mode_) {
     // The selected audio device has been removed or disabled and a new
     // default device has been enabled instead. The current implementation
     // does not to support this sequence of events. Given that Open()
     // and Start() are usually called in one sequence; it should be a very
     // rare event.
     // TODO(henrika): it is possible to extend the functionality here.
     LOG(ERROR) << "Unable to start since the selected default device has "
                   "changed since Open() was called.";
@@ skipping 21 matching lines @@
   // Sanity check: verify that the endpoint buffer is filled with silence.
   UINT32 num_queued_frames = 0;
   audio_client_->GetCurrentPadding(&num_queued_frames);
   DCHECK(num_queued_frames == num_written_frames_);
 
   // Create and start the thread that will drive the rendering by waiting for
   // render events.
   render_thread_.reset(
       new base::DelegateSimpleThread(this, "wasapi_render_thread"));
   render_thread_->Start();
-  if (!render_thread_->HasBeenStarted()) {
-    DLOG(ERROR) << "Failed to start WASAPI render thread.";
-    return;
-  }
 
   // Start streaming data between the endpoint buffer and the audio engine.
   hr = audio_client_->Start();
   if (FAILED(hr)) {
     SetEvent(stop_render_event_.Get());
     render_thread_->Join();
     render_thread_.reset();
     HandleError(hr);
-    return;
   }
-
-  started_ = true;
 }
 
 void WASAPIAudioOutputStream::Stop() {
   DCHECK_EQ(GetCurrentThreadId(), creating_thread_id_);
-  if (!started_)
+  if (!render_thread_.get())
     return;
 
-  // Shut down the render thread.
-  if (stop_render_event_.IsValid()) {
-    SetEvent(stop_render_event_.Get());
-  }
-
   // Stop output audio streaming.
   HRESULT hr = audio_client_->Stop();
   if (FAILED(hr)) {
     DLOG_IF(ERROR, hr != AUDCLNT_E_NOT_INITIALIZED)
         << "Failed to stop output streaming: " << std::hex << hr;
   }
 
   // Wait until the thread completes and perform cleanup.
-  if (render_thread_.get()) {
-    SetEvent(stop_render_event_.Get());
-    render_thread_->Join();
-    render_thread_.reset();
-  }
+  SetEvent(stop_render_event_.Get());
+  render_thread_->Join();
+  render_thread_.reset();
+
+  // Ensure that we don't quit the main thread loop immediately next
+  // time Start() is called.
+  ResetEvent(stop_render_event_.Get());
 
   // Clear source callback, it'll be set again on the next Start() call.
   source_ = NULL;
 
   // Flush all pending data and reset the audio clock stream position to 0.
   hr = audio_client_->Reset();
   if (FAILED(hr)) {
     DLOG_IF(ERROR, hr != AUDCLNT_E_NOT_INITIALIZED)
         << "Failed to reset streaming: " << std::hex << hr;
   }
 
   // Extra safety check to ensure that the buffers are cleared.
   // If the buffers are not cleared correctly, the next call to Start()
   // would fail with AUDCLNT_E_BUFFER_ERROR at IAudioRenderClient::GetBuffer().
   // This check is is only needed for shared-mode streams.
   if (share_mode_ == AUDCLNT_SHAREMODE_SHARED) {
     UINT32 num_queued_frames = 0;
     audio_client_->GetCurrentPadding(&num_queued_frames);
     DCHECK_EQ(0u, num_queued_frames);
   }
-
-  // Ensure that we don't quit the main thread loop immediately next
-  // time Start() is called.
-  ResetEvent(stop_render_event_.Get());
-
-  started_ = false;
 }
 
 void WASAPIAudioOutputStream::Close() {
   DCHECK_EQ(GetCurrentThreadId(), creating_thread_id_);
 
   // It is valid to call Close() before calling open or Start().
   // It is also valid to call Close() after Start() has been called.
   Stop();
 
   if (opened_ && device_enumerator_) {
@@ skipping 302 matching lines @@
     PLOG(ERROR) << "WASAPI rendering failed.";
   }
 
   // Disable MMCSS.
   if (mm_task && !avrt::AvRevertMmThreadCharacteristics(mm_task)) {
     PLOG(WARNING) << "Failed to disable MMCSS";
   }
 }
 
 void WASAPIAudioOutputStream::HandleError(HRESULT err) {
+  CHECK(started() && (GetCurrentThreadId() == render_thread_->tid() ||

[tommi (sloooow) - chröme, 2012/09/20 12:15:38]

I see you've changed the parenthesis, but the check is incorrect now.
This should be:

(started() && GetCurrentThreadId()) == render_thread_->tid()) ||
GetCurrentThreadId() == creating_thread_id_

You should also add a second !started() check to be absolutely sure:

(started() && GetCurrentThreadId()) == render_thread_->tid()) ||
(!started() && GetCurrentThreadId() == creating_thread_id_)

[henrika (OOO until Aug 14), 2012/09/20 12:27:19]

Done.

+        GetCurrentThreadId() == creating_thread_id_));
   NOTREACHED() << "Error code: " << std::hex << err;
   if (source_)
     source_->OnError(this, static_cast<int>(err));
 }
 
 HRESULT WASAPIAudioOutputStream::SetRenderDevice() {
   ScopedComPtr<IMMDeviceEnumerator> device_enumerator;
   ScopedComPtr<IMMDevice> endpoint_device;
 
   // Create the IMMDeviceEnumerator interface.
@@ skipping 416 matching lines @@
   // are now re-initiated and it is now possible to re-start audio rendering.
 
   // Start rendering again using the new default audio endpoint.
   hr = audio_client_->Start();
 
   restart_rendering_mode_ = false;
   return SUCCEEDED(hr);
 }
 
 }  // namespace media