Switch to STA mode for audio thread and WASAPI I/O streams.

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 231 matching lines @@
   if (share_mode_ == AUDCLNT_SHAREMODE_SHARED) {
     if (!CoreAudioUtil::FillRenderEndpointBufferWithSilence(
             audio_client_.get(), audio_render_client_.get())) {
       LOG(ERROR) << "Failed to prepare endpoint buffers with silence.";
       callback->OnError(this);
       return;
     }
   }
   num_written_frames_ = endpoint_buffer_size_frames_;
 
+  if (!MarshalComPointers()) {
+    callback->OnError(this);
+    return;
+  }
+
   // 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()) {
     LOG(ERROR) << "Failed to start WASAPI render thread.";
     StopThread();
     callback->OnError(this);
     return;
   }
 
   // Start streaming data between the endpoint buffer and the audio engine.
+  // TODO(dalecurtis): Do we need a lock on this with STA mode?

[DaleCurtis, 2015/04/22 17:48:54]

Tentatively removed this assuming you POV is correct.

   HRESULT hr = audio_client_->Start();
   if (FAILED(hr)) {
     PLOG(ERROR) << "Failed to start output streaming: " << std::hex << hr;
     StopThread();
     callback->OnError(this);
   }
 }
 
 void WASAPIAudioOutputStream::Stop() {
   DVLOG(1) << "WASAPIAudioOutputStream::Stop()";
   DCHECK_EQ(GetCurrentThreadId(), creating_thread_id_);
   if (!render_thread_)
     return;
 
   // Stop output audio streaming.
+  // TODO(dalecurtis): Do we need a lock on this with STA mode?
   HRESULT hr = audio_client_->Stop();
   if (FAILED(hr)) {
     PLOG(ERROR) << "Failed to stop output streaming: " << std::hex << hr;
     source_->OnError(this);
   }
 
   // Make a local copy of |source_| since StopThread() will clear it.
   AudioSourceCallback* callback = source_;
   StopThread();
 
@@ skipping 36 matching lines @@
   }
   volume_ = volume_float;
 }
 
 void WASAPIAudioOutputStream::GetVolume(double* volume) {
   DVLOG(1) << "GetVolume()";
   *volume = static_cast<double>(volume_);
 }
 
 void WASAPIAudioOutputStream::Run() {
-  ScopedCOMInitializer com_init(ScopedCOMInitializer::kMTA);
+  ScopedCOMInitializer com_init;
 
   // Increase the thread priority.
   render_thread_->SetThreadPriority(base::ThreadPriority::REALTIME_AUDIO);
 
   // Enable MMCSS to ensure that this thread receives prioritized access to
   // CPU resources.
   DWORD task_index = 0;
   HANDLE mm_task = avrt::AvSetMmThreadCharacteristics(L"Pro Audio",
                                                       &task_index);
   bool mmcss_is_ok =
       (mm_task && avrt::AvSetMmThreadPriority(mm_task, AVRT_PRIORITY_CRITICAL));
   if (!mmcss_is_ok) {
     // Failed to enable MMCSS on this thread. It is not fatal but can lead
     // to reduced QoS at high load.
     DWORD err = GetLastError();
     LOG(WARNING) << "Failed to enable MMCSS (error code=" << err << ").";
   }
 
+  // Retrieve COM pointers from the main thread.
+  ScopedComPtr<IAudioClient> thread_audio_client;
+  ScopedComPtr<IAudioRenderClient> thread_audio_render_client;
+  ScopedComPtr<IAudioClock> thread_audio_clock;
+
   HRESULT hr = S_FALSE;
 
   bool playing = true;
-  bool error = false;
+  bool error =
+      !UnmarshalComPointers(&thread_audio_client, &thread_audio_render_client,
+                            &thread_audio_clock);
+
   HANDLE wait_array[] = { stop_render_event_.Get(),
                           audio_samples_render_event_.Get() };
   UINT64 device_frequency = 0;
 
-  // The device frequency is the frequency generated by the hardware clock in
-  // the audio device. The GetFrequency() method reports a constant frequency.
-  hr = audio_clock_->GetFrequency(&device_frequency);
-  error = FAILED(hr);
-  PLOG_IF(ERROR, error) << "Failed to acquire IAudioClock interface: "
-                        << std::hex << hr;
+  if (!error) {
+    // The device frequency is the frequency generated by the hardware clock in
+    // the audio device. The GetFrequency() method reports a constant frequency.
+    hr = thread_audio_clock->GetFrequency(&device_frequency);
+    error = FAILED(hr);
+    PLOG_IF(ERROR, error) << "Failed to acquire IAudioClock interface: "
+                          << std::hex << hr;
+  }
 
   // Keep rendering audio until the stop event or the stream-switch event
   // is signaled. An error event can also break the main thread loop.
   while (playing && !error) {
     // Wait for a close-down event, stream-switch event or a new render event.
     DWORD wait_result = WaitForMultipleObjects(arraysize(wait_array),
                                                wait_array,
                                                FALSE,
                                                INFINITE);
 
     switch (wait_result) {
       case WAIT_OBJECT_0 + 0:
         // |stop_render_event_| has been set.
         playing = false;
         break;
       case WAIT_OBJECT_0 + 1:
         // |audio_samples_render_event_| has been set.
-        error = !RenderAudioFromSource(device_frequency);
+        error = !RenderAudioFromSource(
+                    device_frequency, thread_audio_client.get(),
+                    thread_audio_render_client.get(), thread_audio_clock.get());
         break;
       default:
         error = true;
         break;
     }
   }
 
-  if (playing && error) {
+  if (playing && error && thread_audio_client) {
     // Stop audio rendering since something has gone wrong in our main thread
     // loop. Note that, we are still in a "started" state, hence a Stop() call
     // is required to join the thread properly.
-    audio_client_->Stop();
+    thread_audio_client->Stop();
     PLOG(ERROR) << "WASAPI rendering failed.";
   }
 
   // Disable MMCSS.
   if (mm_task && !avrt::AvRevertMmThreadCharacteristics(mm_task)) {
     PLOG(WARNING) << "Failed to disable MMCSS";
   }
 }
 
-bool WASAPIAudioOutputStream::RenderAudioFromSource(UINT64 device_frequency) {
+bool WASAPIAudioOutputStream::RenderAudioFromSource(
+    UINT64 device_frequency,
+    IAudioClient* thread_audio_client,

[tommi (sloooow) - chröme, 2015/04/22 10:31:52]

nit: Does the 'thread_' prefix add context?  I think we can just skip it, but
it's up to you.

[DaleCurtis, 2015/04/22 17:48:54]

Done.

+    IAudioRenderClient* thread_audio_render_client,
+    IAudioClock* thread_audio_clock) {
   TRACE_EVENT0("audio", "RenderAudioFromSource");
 
   HRESULT hr = S_FALSE;
   UINT32 num_queued_frames = 0;
   uint8* audio_data = NULL;
 
   // Contains how much new data we can write to the buffer without
   // the risk of overwriting previously written data that the audio
   // engine has not yet read from the buffer.
   size_t num_available_frames = 0;
 
   if (share_mode_ == AUDCLNT_SHAREMODE_SHARED) {
     // Get the padding value which represents the amount of rendering
     // data that is queued up to play in the endpoint buffer.
-    hr = audio_client_->GetCurrentPadding(&num_queued_frames);
+    hr = thread_audio_client->GetCurrentPadding(&num_queued_frames);
     num_available_frames =
         endpoint_buffer_size_frames_ - num_queued_frames;
     if (FAILED(hr)) {
       DLOG(ERROR) << "Failed to retrieve amount of available space: "
                   << std::hex << hr;
       return false;
     }
   } else {
     // While the stream is running, the system alternately sends one
     // buffer or the other to the client. This form of double buffering
@@ skipping 21 matching lines @@
   // fill up the available area in the endpoint buffer.
   // |num_packets| will always be one for exclusive-mode streams and
   // will be one in most cases for shared mode streams as well.
   // However, we have found that two packets can sometimes be
   // required.
   size_t num_packets = (num_available_frames / packet_size_frames_);
 
   for (size_t n = 0; n < num_packets; ++n) {
     // Grab all available space in the rendering endpoint buffer
     // into which the client can write a data packet.
-    hr = audio_render_client_->GetBuffer(packet_size_frames_,
-                                         &audio_data);
+    hr =
+        thread_audio_render_client->GetBuffer(packet_size_frames_, &audio_data);
     if (FAILED(hr)) {
       DLOG(ERROR) << "Failed to use rendering audio buffer: "
                  << std::hex << hr;
       return false;
     }
 
     // Derive the audio delay which corresponds to the delay between
     // a render event and the time when the first audio sample in a
     // packet is played out through the speaker. This delay value
     // can typically be utilized by an acoustic echo-control (AEC)
     // unit at the render side.
     UINT64 position = 0;
     uint32 audio_delay_bytes = 0;
-    hr = audio_clock_->GetPosition(&position, NULL);
+    hr = thread_audio_clock->GetPosition(&position, NULL);
     if (SUCCEEDED(hr)) {
       // Stream position of the sample that is currently playing
       // through the speaker.
       double pos_sample_playing_frames = format_.Format.nSamplesPerSec *
           (static_cast<double>(position) / device_frequency);
 
       // Stream position of the last sample written to the endpoint
       // buffer. Note that, the packet we are about to receive in
       // the upcoming callback is also included.
       size_t pos_last_sample_written_frames =
@@ skipping 19 matching lines @@
     const int bytes_per_sample = format_.Format.wBitsPerSample >> 3;
     audio_bus_->Scale(volume_);
     audio_bus_->ToInterleaved(
         frames_filled, bytes_per_sample, audio_data);
 
 
     // Release the buffer space acquired in the GetBuffer() call.
     // Render silence if we were not able to fill up the buffer totally.
     DWORD flags = (num_filled_bytes < packet_size_bytes_) ?
         AUDCLNT_BUFFERFLAGS_SILENT : 0;
-    audio_render_client_->ReleaseBuffer(packet_size_frames_, flags);
+    thread_audio_render_client->ReleaseBuffer(packet_size_frames_, flags);
 
     num_written_frames_ += packet_size_frames_;
   }
 
   return true;
 }
 
 HRESULT WASAPIAudioOutputStream::ExclusiveModeInitialization(
     IAudioClient* client, HANDLE event_handle, uint32* endpoint_buffer_size) {
   DCHECK_EQ(share_mode_, AUDCLNT_SHAREMODE_EXCLUSIVE);
@@ skipping 84 matching lines @@
     render_thread_.reset();
 
     // Ensure that we don't quit the main thread loop immediately next
     // time Start() is called.
     ResetEvent(stop_render_event_.Get());
   }
 
   source_ = NULL;
 }
 
+bool WASAPIAudioOutputStream::MarshalComPointers() {

[tommi (sloooow) - chröme, 2015/04/22 10:31:52]

Can we add a thread checker for these methods?  There's no lock etc for
com_stream_ etc, so it would be nice to have that.

[DaleCurtis, 2015/04/22 16:08:23]

I'll see if I can. I forget if the unit tests try to make any calls directly
into the render/capture threads.

[DaleCurtis, 2015/04/22 17:48:54]

Done.

+  HRESULT hr = CreateStreamOnHGlobal(NULL, TRUE, com_stream_.Receive());

[tommi (sloooow) - chröme, 2015/04/22 10:31:52]

what about using a local variable for the stream here and assign to com_stream_
at the end of this function if all ends well?  Otherwise I worry about the risk
of not releasing com_stream_ for some early return that might get added down the
line and possibly cause subtle errors.  Same for UnmarshalComPointers.

[DaleCurtis, 2015/04/22 16:08:23]

Good idea, I'll do this.

[DaleCurtis, 2015/04/22 17:48:54]

Done.

+  if (FAILED(hr)) {
+    DLOG(ERROR) << "Failed to create stream for marshaling COM pointers.";
+    return false;
+  }
+
+  hr = CoMarshalInterface(com_stream_.get(), __uuidof(IAudioClient),

[tommi (sloooow) - chröme, 2015/04/22 10:31:52]

was there a particular reason you decided to go with CoMarshalInterface and not
the GIT? (IGlobalInterfaceTable)

[DaleCurtis, 2015/04/22 16:08:23]

Yes, but maybe not good ones, as I basically just found a bunch of internet
tutorials.  The CoMarshalInterface method looked like a better scoped transfer
mechanism and examples used far less code.

+                          audio_client_.get(), MSHCTX_INPROC, NULL,
+                          MSHLFLAGS_NORMAL);
+  if (FAILED(hr)) {
+    DLOG(ERROR) << "Marshal failed for IAudioClient: " << std::hex << hr;
+    com_stream_.Release();
+    return false;
+  }
+
+  hr = CoMarshalInterface(com_stream_.get(), __uuidof(IAudioRenderClient),
+                          audio_render_client_.get(), MSHCTX_INPROC, NULL,
+                          MSHLFLAGS_NORMAL);
+  if (FAILED(hr)) {
+    DLOG(ERROR) << "Marshal failed for IAudioRenderClient: " << std::hex << hr;
+    com_stream_.Release();
+    return false;
+  }
+
+  hr = CoMarshalInterface(com_stream_.get(), __uuidof(IAudioClock),
+                          audio_clock_.get(), MSHCTX_INPROC, NULL,
+                          MSHLFLAGS_NORMAL);
+  if (FAILED(hr)) {
+    DLOG(ERROR) << "Marshal failed for IAudioClock: " << std::hex << hr;
+    com_stream_.Release();
+    return false;
+  }
+
+  LARGE_INTEGER pos = {0};
+  hr = com_stream_->Seek(pos, STREAM_SEEK_SET, NULL);
+  if (FAILED(hr)) {
+    DLOG(ERROR) << "Failed to seek IStream for marshaling: " << std::hex << hr;
+    com_stream_.Release();
+    return false;
+  }
+
+  return true;
+}
+
+bool WASAPIAudioOutputStream::UnmarshalComPointers(
+    ScopedComPtr<IAudioClient>* audio_client,
+    ScopedComPtr<IAudioRenderClient>* audio_render_client,
+    ScopedComPtr<IAudioClock>* audio_clock) {
+  HRESULT hr = CoUnmarshalInterface(com_stream_.get(), __uuidof(IAudioClient),

[tommi (sloooow) - chröme, 2015/04/22 10:31:52]

nit: would be nice to detach com_stream_ here to a local variable and skip
calling com_stream_.Release() at every return point:

ScopedComPtr<IStream> stream(com_stream_.Detach());

[DaleCurtis, 2015/04/22 17:48:54]

Done.

+                                    audio_client->ReceiveVoid());
+  if (FAILED(hr)) {
+    DLOG(ERROR) << "Unmarshal failed IAudioClient: " << std::hex << hr;
+    com_stream_.Release();
+    return false;
+  }
+
+  hr = CoUnmarshalInterface(com_stream_.get(), __uuidof(IAudioRenderClient),
+                            audio_render_client->ReceiveVoid());
+  if (FAILED(hr)) {
+    DLOG(ERROR) << "Unmarshal failed IAudioRenderClient: " << std::hex << hr;
+    com_stream_.Release();
+    return false;
+  }
+
+  hr = CoUnmarshalInterface(com_stream_.get(), __uuidof(IAudioClock),
+                            audio_clock->ReceiveVoid());
+  if (FAILED(hr))
+    DLOG(ERROR) << "Unmarshal failed IAudioClock: " << std::hex << hr;
+  com_stream_.Release();
+  return SUCCEEDED(hr);
+}
+
 }  // namespace media