Bug 1135878 - Simply post-error cleanup logic in WASAPI cubeb backend. r=padenot
authorMatthew Gregan <kinetik@flim.org>
Tue, 24 Feb 2015 13:42:33 +1300
changeset 230501 65a968102d6dd986563e38525cbe195282ea9e23
parent 230500 aff54769dd05ca17cc8df37896c85ec04c692428
child 230502 be9b4a3b01ab2719af5b2f6388962ed0dad3fd3f
push id28327
push userkwierso@gmail.com
push dateTue, 24 Feb 2015 23:13:22 +0000
treeherdermozilla-central@a9f3cf7f7ec9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspadenot
bugs1135878
milestone39.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 1135878 - Simply post-error cleanup logic in WASAPI cubeb backend. 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 cacaae79dd8b7220202d0dfe3f889d55e23a77a5.
+The git commit ID used was 6de5d3e488d808dd925ae0885a7552fc0a25b449.
--- a/media/libcubeb/src/cubeb_wasapi.cpp
+++ b/media/libcubeb/src/cubeb_wasapi.cpp
@@ -515,17 +515,24 @@ wasapi_stream_render_loop(LPVOID stream)
     case WAIT_OBJECT_0 + 1: { /* reconfigure */
       /* Close the stream */
       stm->client->Stop();
       {
         auto_lock lock(stm->stream_reset_lock);
         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);
+        int r = setup_wasapi_stream(stm);
+        if (r != CUBEB_OK) {
+          /* Don't destroy the stream here, since we expect the caller to do
+             so after the error has propagated via the state callback. */
+          is_playing = false;
+          hr = -1;
+          continue;
+        }
       }
       stm->client->Start();
       break;
     }
     case WAIT_OBJECT_0 + 2: { /* refill */
       UINT32 padding;
 
       hr = stm->client->GetCurrentPadding(&padding);
@@ -573,17 +580,17 @@ wasapi_stream_render_loop(LPVOID stream)
       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);
+    stm->state_callback(stm, stm->user_ptr, CUBEB_STATE_ERROR);
   }
 
   stm->context->revert_mm_thread_characteristics(mmcss_handle);
 
   return 0;
 }
 
 void wasapi_destroy(cubeb * context);
@@ -962,41 +969,35 @@ int setup_wasapi_stream(cubeb_stream * s
     return CUBEB_ERROR;
   }
 
   XASSERT(!stm->client && "WASAPI stream already setup, close it first.");
 
   hr = get_default_endpoint(&device);
   if (FAILED(hr)) {
     LOG("Could not get default endpoint, error: %x\n", hr);
-    stm->stream_reset_lock->leave();
-    wasapi_stream_destroy(stm);
     return CUBEB_ERROR;
   }
 
   /* Get a client. We will get all other interfaces we need from
    * this pointer. */
   hr = device->Activate(__uuidof(IAudioClient),
                         CLSCTX_INPROC_SERVER,
                         NULL, (void **)&stm->client);
   SafeRelease(device);
   if (FAILED(hr)) {
     LOG("Could not activate the device to get an audio client: error: %x\n", hr);
-    stm->stream_reset_lock->leave();
-    wasapi_stream_destroy(stm);
     return CUBEB_ERROR;
   }
 
   /* We have to distinguish between the format the mixer uses,
    * and the format the stream we want to play uses. */
   hr = stm->client->GetMixFormat(&mix_format);
   if (FAILED(hr)) {
     LOG("Could not fetch current mix format from the audio client: error: %x\n", hr);
-    stm->stream_reset_lock->leave();
-    wasapi_stream_destroy(stm);
     return CUBEB_ERROR;
   }
 
   handle_channel_layout(stm, &mix_format, &stm->stream_params);
 
   /* Shared mode WASAPI always supports float32 sample format, so this
    * is safe. */
   stm->mix_params.format = CUBEB_SAMPLE_FLOAT32NE;
@@ -1010,73 +1011,61 @@ int setup_wasapi_stream(cubeb_stream * s
                                0,
                                mix_format,
                                NULL);
 
   CoTaskMemFree(mix_format);
 
   if (FAILED(hr)) {
     LOG("Unable to initialize audio client: %x.\n", hr);
-    stm->stream_reset_lock->leave();
-    wasapi_stream_destroy(stm);
     return CUBEB_ERROR;
   }
 
   hr = stm->client->GetBufferSize(&stm->buffer_frame_count);
   if (FAILED(hr)) {
     LOG("Could not get the buffer size from the client %x.\n", hr);
-    stm->stream_reset_lock->leave();
-    wasapi_stream_destroy(stm);
     return CUBEB_ERROR;
   }
 
   if (should_upmix(stm) || should_downmix(stm)) {
     stm->mix_buffer = (float *) malloc(frames_to_bytes_before_mix(stm, stm->buffer_frame_count));
   }
 
   hr = stm->client->SetEventHandle(stm->refill_event);
   if (FAILED(hr)) {
     LOG("Could set the event handle for the client %x.\n", hr);
-    stm->stream_reset_lock->leave();
-    wasapi_stream_destroy(stm);
     return CUBEB_ERROR;
   }
 
   hr = stm->client->GetService(__uuidof(IAudioRenderClient),
                                (void **)&stm->render_client);
   if (FAILED(hr)) {
     LOG("Could not get the render client %x.\n", hr);
-    stm->stream_reset_lock->leave();
-    wasapi_stream_destroy(stm);
     return CUBEB_ERROR;
   }
 
   hr = stm->client->GetService(__uuidof(IAudioStreamVolume),
                                (void **)&stm->audio_stream_volume);
   if (FAILED(hr)) {
     LOG("Could not get the IAudioStreamVolume %x.\n", hr);
-    stm->stream_reset_lock->leave();
-    wasapi_stream_destroy(stm);
     return CUBEB_ERROR;
   }
 
   /* If we are playing a mono stream, we only resample one channel,
    * and copy it over, so we are always resampling the number
    * of channels of the stream, not the number of channels
    * that WASAPI wants. */
   stm->resampler = cubeb_resampler_create(stm, stm->stream_params,
                                           stm->mix_params.rate,
                                           stm->data_callback,
                                           stm->buffer_frame_count,
                                           stm->user_ptr,
                                           CUBEB_RESAMPLER_QUALITY_DESKTOP);
   if (!stm->resampler) {
     LOG("Could not get a resampler\n");
-    stm->stream_reset_lock->leave();
-    wasapi_stream_destroy(stm);
     return CUBEB_ERROR;
   }
 
   return CUBEB_OK;
 }
 
 int
 wasapi_stream_init(cubeb * context, cubeb_stream ** stream,
@@ -1126,24 +1115,25 @@ wasapi_stream_init(cubeb * context, cube
   stm->refill_event = CreateEvent(NULL, 0, 0, NULL);
   if (!stm->refill_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
+    /* Locking here is not strictly necessary, because we don't have a
        notification client that can reset the stream yet, but it lets us
        assert that the lock is held in the function. */
     auto_lock lock(stm->stream_reset_lock);
     rv = setup_wasapi_stream(stm);
-    if (rv != CUBEB_OK) {
-      return rv;
-    }
+  }
+  if (rv != CUBEB_OK) {
+    wasapi_stream_destroy(stm);
+    return rv;
   }
 
   hr = register_notification_client(stm);
   if (FAILED(hr)) {
     /* this is not fatal, we can still play audio, but we won't be able
      * to keep using the default audio endpoint if it changes. */
     LOG("failed to register notification client, %x\n", hr);
   }