Bug 1134078 - Don't restart stopped cubeb streams when handling device change notifications. r=padenot
authorMatthew Gregan <kinetik@flim.org>
Wed, 18 Feb 2015 16:06:55 +1300
changeset 229789 425f602236255e8c0a2f23d08ba6d0aeb7f9cb85
parent 229788 a58a44e992d2918fe9af10dae182cb65a2b24046
child 229790 99f5101d6cf18479f300d29725026094db957673
push id55804
push usermgregan@mozilla.com
push dateThu, 19 Feb 2015 06:35:35 +0000
treeherdermozilla-inbound@c841c87a7791 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspadenot
bugs1134078
milestone38.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1134078 - Don't restart stopped cubeb streams when handling device change notifications. r=padenot
media/libcubeb/README_MOZILLA
media/libcubeb/src/cubeb_wasapi.cpp
--- a/media/libcubeb/README_MOZILLA
+++ b/media/libcubeb/README_MOZILLA
@@ -1,8 +1,8 @@
 The source from this directory was copied from the cubeb 
 git repository using the update.sh script.  The only changes
 made were those applied by update.sh and the addition of
 Makefile.in build files for the Mozilla build system.
 
 The cubeb git repository is: git://github.com/kinetiknz/cubeb.git
 
-The git commit ID used was 66aa55bb3b883381a73ad12d20688b932858be28.
+The git commit ID used was a74c790e53a99797c59c4a606e1aa1f16987b18e.
--- a/media/libcubeb/src/cubeb_wasapi.cpp
+++ b/media/libcubeb/src/cubeb_wasapi.cpp
@@ -110,25 +110,25 @@ public:
   {
 #ifdef DEBUG
     /* GetCurrentThreadId cannot return 0: it is not a the valid thread id */
     owner = 0;
 #endif
     LeaveCriticalSection(&critical_section);
   }
 
-#ifdef DEBUG
   /* This is guaranteed to have the good behaviour if it succeeds. The behaviour
    * is undefined otherwise. */
   void assert_current_thread_owns()
   {
+#ifdef DEBUG
     /* This implies owner != 0, because GetCurrentThreadId cannot return 0. */
     assert(owner == GetCurrentThreadId());
+#endif
   }
-#endif
 
 private:
   CRITICAL_SECTION critical_section;
 #ifdef DEBUG
   DWORD owner;
 #endif
 };
 
@@ -187,17 +187,17 @@ private:
 };
 
 typedef HANDLE (WINAPI *set_mm_thread_characteristics_function)(
                                       const char * TaskName, LPDWORD TaskIndex);
 typedef BOOL (WINAPI *revert_mm_thread_characteristics_function)(HANDLE handle);
 
 extern cubeb_ops const wasapi_ops;
 
-int wasapi_stream_stop(cubeb_stream * stm);
+int stream_stop(cubeb_stream * stm, bool * was_running);
 int wasapi_stream_start(cubeb_stream * stm);
 void close_wasapi_stream(cubeb_stream * stm);
 int setup_wasapi_stream(cubeb_stream * stm);
 
 }
 
 struct cubeb
 {
@@ -305,40 +305,39 @@ public:
   wasapi_endpoint_notification_client(cubeb_stream * stm)
     : ref_count(1)
     , stm(stm)
   { }
 
   HRESULT STDMETHODCALLTYPE
   OnDefaultDeviceChanged(EDataFlow flow, ERole role, LPCWSTR device_id)
   {
-    /* we don't support capture for now. */
-    if (flow == eCapture) {
-      return S_OK;
-    }
-    /* all our streams are eMultimedia for now */
-    if (role != eMultimedia) {
+    /* we only support a single stream type for now. */
+    if (flow != eRender && role != eMultimedia) {
       return S_OK;
     }
 
     auto_com com;
     if (!com.ok()) {
       return E_FAIL;
     }
 
     /* Close the stream */
-    wasapi_stream_stop(stm);
+    bool was_running;
     {
       auto_lock lock(stm->stream_reset_lock);
+      stream_stop(stm, &was_running);
       close_wasapi_stream(stm);
       /* Reopen a stream and start it immediately. This will automatically pick the
        * new default device for this role. */
       setup_wasapi_stream(stm);
     }
-    wasapi_stream_start(stm);
+    if (was_running) {
+      wasapi_stream_start(stm);
+    }
 
     return S_OK;
   }
 
   /* The remaining methods are not implemented, they simply log when called (if
    * log is enabled), for debugging. */
   HRESULT STDMETHODCALLTYPE OnDeviceAdded(LPCWSTR device_id)
   {
@@ -527,17 +526,17 @@ wasapi_stream_render_loop(LPVOID stream)
     LOG("Unable to use mmcss to bump the render thread priority: %x\n", GetLastError());
   }
 
 
   while (is_playing) {
     DWORD waitResult = WaitForMultipleObjects(ARRAY_LENGTH(wait_array),
                                               wait_array,
                                               FALSE,
-                                              INFINITE);
+                                              1000);
 
     switch (waitResult) {
     case WAIT_OBJECT_0: { /* shutdown */
       is_playing = false;
       /* We don't check if the drain is actually finished here, we just want to
        * shutdown. */
       if (stm->draining) {
         stm->state_callback(stm, stm->user_ptr, CUBEB_STATE_DRAINED);
@@ -580,16 +579,19 @@ wasapi_stream_render_loop(LPVOID stream)
           is_playing = false;
         }
       } else {
         LOG("failed to get buffer.\n");
         is_playing = false;
       }
     }
     break;
+    case WAIT_TIMEOUT:
+      assert(stm->shutdown_event == wait_array[0]);
+      break;
     default:
       LOG("case %d not handled in render loop.", waitResult);
       abort();
     }
   }
 
   if (FAILED(hr)) {
     stm->state_callback(stm, stm->user_ptr, CUBEB_STATE_STOPPED);
@@ -725,16 +727,38 @@ int wasapi_init(cubeb ** context, char c
 
   *context = ctx;
 
   return CUBEB_OK;
 }
 }
 
 namespace {
+void stop_and_join_render_thread(cubeb_stream * stm)
+{
+  if (!stm->thread) {
+    return;
+  }
+
+  BOOL ok = SetEvent(stm->shutdown_event);
+  if (!ok) {
+    LOG("Destroy SetEvent failed: %d\n", GetLastError());
+  }
+
+  DWORD r = WaitForSingleObject(stm->thread, INFINITE);
+  if (r == WAIT_FAILED) {
+    LOG("Destroy WaitForSingleObject on thread failed: %d\n", GetLastError());
+  }
+
+  CloseHandle(stm->thread);
+  stm->thread = NULL;
+
+  CloseHandle(stm->shutdown_event);
+  stm->shutdown_event = 0;
+}
 
 void wasapi_destroy(cubeb * context)
 {
   if (context->mmcss_module) {
     FreeLibrary(context->mmcss_module);
   }
   free(context);
 }
@@ -942,19 +966,17 @@ handle_channel_layout(cubeb_stream * stm
 }
 
 int setup_wasapi_stream(cubeb_stream * stm)
 {
   HRESULT hr;
   IMMDevice * device;
   WAVEFORMATEX * mix_format;
 
-#ifdef DEBUG
   stm->stream_reset_lock->assert_current_thread_owns();
-#endif
 
   auto_com com;
   if (!com.ok()) {
     return CUBEB_ERROR;
   }
 
   assert(!stm->client && "WASAPI stream already setup, close it first.");
 
@@ -1105,27 +1127,18 @@ wasapi_stream_init(cubeb * context, cube
   stm->client = NULL;
   stm->render_client = NULL;
   stm->audio_stream_volume = NULL;
   stm->device_enumerator = NULL;
   stm->notification_client = NULL;
 
   stm->stream_reset_lock = new owned_critical_section();
 
-  stm->shutdown_event = CreateEvent(NULL, 0, 0, NULL);
   stm->refill_event = CreateEvent(NULL, 0, 0, NULL);
-
-  if (!stm->shutdown_event) {
-    LOG("Can't create the shutdown event, error: %x\n", GetLastError());
-    wasapi_stream_destroy(stm);
-    return CUBEB_ERROR;
-  }
-
   if (!stm->refill_event) {
-    SafeRelease(stm->shutdown_event);
     LOG("Can't create the refill event, error: %x\n", GetLastError());
     wasapi_stream_destroy(stm);
     return CUBEB_ERROR;
   }
 
   {
     /* Locking here is not stricly necessary, because we don't have a
        notification client that can reset the stream yet, but it lets us
@@ -1148,19 +1161,17 @@ wasapi_stream_init(cubeb * context, cube
 
   return CUBEB_OK;
 }
 
 void close_wasapi_stream(cubeb_stream * stm)
 {
   assert(stm);
 
-#ifdef DEBUG
   stm->stream_reset_lock->assert_current_thread_owns();
-#endif
 
   SafeRelease(stm->client);
   stm->client = NULL;
 
   SafeRelease(stm->render_client);
   stm->render_client = NULL;
 
   if (stm->resampler) {
@@ -1173,47 +1184,41 @@ void close_wasapi_stream(cubeb_stream * 
 }
 
 void wasapi_stream_destroy(cubeb_stream * stm)
 {
   assert(stm);
 
   unregister_notification_client(stm);
 
-  if (stm->thread) {
-    BOOL ok = SetEvent(stm->shutdown_event);
-    if (!ok) {
-      LOG("Destroy SetEvent failed: %d\n", GetLastError());
-    }
-    DWORD r = WaitForSingleObject(stm->thread, INFINITE);
-    if (r == WAIT_FAILED) {
-      LOG("Destroy WaitForSingleObject on thread failed: %d\n", GetLastError());
-    }
-    CloseHandle(stm->thread);
-    stm->thread = 0;
-  }
+  stop_and_join_render_thread(stm);
 
-  SafeRelease(stm->shutdown_event);
   SafeRelease(stm->refill_event);
 
   {
     auto_lock lock(stm->stream_reset_lock);
     close_wasapi_stream(stm);
   }
 
   delete stm->stream_reset_lock;
 
   free(stm);
 }
 
 int wasapi_stream_start(cubeb_stream * stm)
 {
   auto_lock lock(stm->stream_reset_lock);
 
-  assert(stm && !stm->thread);
+  assert(stm && !stm->thread && !stm->shutdown_event);
+
+  stm->shutdown_event = CreateEvent(NULL, 0, 0, NULL);
+  if (!stm->shutdown_event) {
+    LOG("Can't create the shutdown event, error: %x\n", GetLastError());
+    return CUBEB_ERROR;
+  }
 
   stm->thread = (HANDLE) _beginthreadex(NULL, 256 * 1024, wasapi_stream_render_loop, stm, STACK_SIZE_PARAM_IS_A_RESERVATION, NULL);
   if (stm->thread == NULL) {
     LOG("could not create WASAPI render thread.\n");
     return CUBEB_ERROR;
   }
 
   HRESULT hr = stm->client->Start();
@@ -1222,49 +1227,49 @@ int wasapi_stream_start(cubeb_stream * s
     return CUBEB_ERROR;
   } else {
     stm->state_callback(stm, stm->user_ptr, CUBEB_STATE_STARTED);
   }
 
   return FAILED(hr) ? CUBEB_ERROR : CUBEB_OK;
 }
 
-int wasapi_stream_stop(cubeb_stream * stm)
+int stream_stop(cubeb_stream * stm, bool * was_running)
 {
-  assert(stm && stm->shutdown_event);
+  assert(stm);
 
-  auto_lock lock(stm->stream_reset_lock);
-
-  BOOL ok = SetEvent(stm->shutdown_event);
-  if (!ok) {
-    LOG("Stop SetEvent failed: %d\n", GetLastError());
-  }
+  stm->stream_reset_lock->assert_current_thread_owns();
 
   HRESULT hr = stm->client->Stop();
   if (FAILED(hr)) {
     LOG("could not stop AudioClient\n");
   }
+  if (was_running) {
+    *was_running = hr == S_OK;
+    assert(*was_running == !!stm->thread);
+  }
 
-  if (stm->thread) {
+  {
     auto_unlock lock(stm->stream_reset_lock);
-    DWORD r = WaitForSingleObject(stm->thread, INFINITE);
-    if (r == WAIT_FAILED) {
-      LOG("Stop WaitForSingleObject on thread failed: %d\n", GetLastError());
-    }
-    CloseHandle(stm->thread);
-    stm->thread = NULL;
+    stop_and_join_render_thread(stm);
   }
 
   if (SUCCEEDED(hr)) {
     stm->state_callback(stm, stm->user_ptr, CUBEB_STATE_STOPPED);
   }
 
   return FAILED(hr) ? CUBEB_ERROR : CUBEB_OK;
 }
 
+int wasapi_stream_stop(cubeb_stream * stm)
+{
+  auto_lock lock(stm->stream_reset_lock);
+  return stream_stop(stm, NULL);
+}
+
 int wasapi_stream_get_position(cubeb_stream * stm, uint64_t * position)
 {
   assert(stm && position);
 
   *position = clock_get(stm);
 
   return CUBEB_OK;
 }