Change the WinMM backend so that it doesn't call the user callback during initialization, and other fixes

.github/workflows/build.yml

@@ -25,6 +25,17 @@ jobs:
       run: pulseaudio -D --start
       if: matrix.os == 'ubuntu-20.04'
 
+    - name: Install virtual audio devices (Windows)
+      run: git clone https://github.com/LABSN/sound-ci-helpers && powershell sound-ci-helpers/windows/setup_sound.ps1
+      if: ${{ matrix.os == 'windows-2019' }}
+
+    - name: Allow microphone access to all apps (Windows)
+      shell: pwsh
+      run: |
+        New-Item -Path "HKLM:\SOFTWARE\Policies\Microsoft\Windows\AppPrivacy\"
+        New-ItemProperty -Path "HKLM:\SOFTWARE\policies\microsoft\windows\appprivacy" -Name "LetAppsAccessMicrophone" -Value "0x00000001"  -PropertyType "dword"
+      if: ${{ matrix.os == 'windows-2019' }}
+
     - name: Configure CMake
       shell: bash
       run: cmake -S . -B build -DCMAKE_BUILD_TYPE=$BUILD_TYPE
@@ -36,7 +47,11 @@ jobs:
     - name: Test
       shell: bash
       run: (cd build && ctest -V)
-      if: ${{ matrix.os == 'ubuntu-20.04' || matrix.os == 'macos-11' }}
+
+    - name: Test winmm
+      shell: bash
+      run: (cd build && CUBEB_BACKEND=winmm ctest -V)
+      if: ${{ matrix.os == 'windows-2019' }}
 
   build-android:
     runs-on: ubuntu-20.04

src/cubeb_winmm.c

@@ -105,10 +105,13 @@ struct cubeb_stream {
   int free_buffers;
   int shutdown;
   int draining;
+  int error;
   HANDLE event;
   HWAVEOUT waveout;
   CRITICAL_SECTION lock;
   uint64_t written;
+  /* number of frames written during preroll */
+  uint64_t position_base;
   float soft_volume;
   /* For position wrap-around handling: */
   size_t frame_size;
@@ -150,6 +153,14 @@ winmm_get_next_buffer(cubeb_stream * stm)
   return hdr;
 }
 
+static long
+preroll_callback(cubeb_stream * stream, void * user, const void * inputbuffer,
+                 void * outputbuffer, long nframes)
+{
+  memset((uint8_t *)outputbuffer, 0, nframes * bytes_per_frame(stream->params));
+  return nframes;
+}
+
 static void
 winmm_refill_stream(cubeb_stream * stm)
 {
@@ -158,13 +169,20 @@ winmm_refill_stream(cubeb_stream * stm)
   long wanted;
   MMRESULT r;
 
+  ALOG("winmm_refill_stream");
+
   EnterCriticalSection(&stm->lock);
+  if (stm->error) {
+    LeaveCriticalSection(&stm->lock);
+    return;
+  }
   stm->free_buffers += 1;
   XASSERT(stm->free_buffers > 0 && stm->free_buffers <= NBUFS);
 
   if (stm->draining) {
     LeaveCriticalSection(&stm->lock);
     if (stm->free_buffers == NBUFS) {
+      ALOG("winmm_refill_stream draining");
       stm->state_callback(stm, stm->user_ptr, CUBEB_STATE_DRAINED);
     }
     SetEvent(stm->event);
@@ -187,9 +205,10 @@ winmm_refill_stream(cubeb_stream * stm)
   got = stm->data_callback(stm, stm->user_ptr, NULL, hdr->lpData, wanted);
   EnterCriticalSection(&stm->lock);
   if (got < 0) {
+    stm->error = 1;
     LeaveCriticalSection(&stm->lock);
-    /* XXX handle this case */
-    XASSERT(0);
+    SetEvent(stm->event);
+    stm->state_callback(stm, stm->user_ptr, CUBEB_STATE_ERROR);
     return;
   } else if (got < wanted) {
     stm->draining = 1;
@@ -224,6 +243,8 @@ winmm_refill_stream(cubeb_stream * stm)
     return;
   }
 
+  ALOG("winmm_refill_stream %ld frames", got);
+
   LeaveCriticalSection(&stm->lock);
 }
 
@@ -486,7 +507,11 @@ winmm_stream_init(cubeb * context, cubeb_stream ** stream,
 
   stm->params = *output_stream_params;
 
-  stm->data_callback = data_callback;
+  // Data callback is set to the user-provided data callback after
+  // the initialization and potential preroll callback calls are done, because
+  // cubeb users don't expect the data callback to be called during
+  // initialization.
+  stm->data_callback = preroll_callback;
   stm->state_callback = state_callback;
   stm->user_ptr = user_ptr;
   stm->written = 0;
@@ -553,9 +578,18 @@ winmm_stream_init(cubeb * context, cubeb_stream ** stream,
   stm->frame_size = bytes_per_frame(stm->params);
   stm->prev_pos_lo_dword = 0;
   stm->pos_hi_dword = 0;
+  // Set the user data callback now that preroll has finished.
+  stm->data_callback = data_callback;
+  stm->position_base = 0;
+
+  // Offset the position by the number of frames written during preroll.
+  stm->position_base = stm->written;
+  stm->written = 0;
 
   *stream = stm;
 
+  LOG("winmm_stream_init OK");
+
   return CUBEB_OK;
 }
 
@@ -585,7 +619,7 @@ winmm_stream_destroy(cubeb_stream * stm)
     LeaveCriticalSection(&stm->lock);
 
     /* Wait for all blocks to complete. */
-    while (device_valid && enqueued > 0) {
+    while (device_valid && enqueued > 0 && !stm->error) {
       DWORD rv = WaitForSingleObject(stm->event, INFINITE);
       XASSERT(rv == WAIT_OBJECT_0);
 
@@ -774,7 +808,17 @@ winmm_stream_get_position(cubeb_stream * stm, uint64_t * position)
     return CUBEB_ERROR;
   }
 
-  *position = update_64bit_position(stm, time.u.cb) / stm->frame_size;
+  uint64_t position_not_adjusted =
+      update_64bit_position(stm, time.u.cb) / stm->frame_size;
+
+  // Subtract the number of frames that were written while prerolling, during
+  // initialization.
+  if (position_not_adjusted < stm->position_base) {
+    *position = 0;
+  } else {
+    *position = position_not_adjusted - stm->position_base;
+  }
+
   LeaveCriticalSection(&stm->lock);
 
   return CUBEB_OK;
@@ -787,17 +831,12 @@ winmm_stream_get_latency(cubeb_stream * stm, uint32_t * latency)
   MMTIME time;
   uint64_t written, position;
 
-  EnterCriticalSection(&stm->lock);
-  /* See the long comment above for why not just use TIME_SAMPLES here. */
-  time.wType = TIME_BYTES;
-  r = waveOutGetPosition(stm->waveout, &time, sizeof(time));
-
-  if (r != MMSYSERR_NOERROR || time.wType != TIME_BYTES) {
-    LeaveCriticalSection(&stm->lock);
-    return CUBEB_ERROR;
+  int rv = winmm_stream_get_position(stm, &position);
+  if (rv != CUBEB_OK) {
+    return rv;
   }
 
-  position = update_64bit_position(stm, time.u.cb);
+  EnterCriticalSection(&stm->lock);
   written = stm->written;
   LeaveCriticalSection(&stm->lock);
 

test/common.h

@@ -48,7 +48,34 @@ typedef struct {
   uint32_t const layout;
 } layout_info;
 
-inline int has_available_input_device(cubeb * ctx)
+struct backend_caps {
+  const char* id;
+  const int input_capabilities;
+};
+
+
+// This static table allows knowing if a backend has audio input capabilities.
+// We don't rely on opening a stream and checking if it works, because this
+// would make the test skip the tests that make use of audio input, if a
+// particular backend has a bug that causes a failure during audio input stream
+// creation 
+static backend_caps backend_capabilities[] = {
+  {"sun", 1},
+  {"wasapi", 1},
+  {"kai", 1},
+  {"audiounit", 1},
+  {"audiotrack", 0},
+  {"opensl", 1},
+  {"aaudio", 1},
+  {"jack", 1},
+  {"pulse", 1},
+  {"sndio", 1},
+  {"oss", 1},
+  {"winmm", 0},
+  {"alsa", 1},
+};
+
+inline int can_run_audio_input_test(cubeb * ctx)
 {
   cubeb_device_collection devices;
   int input_device_available = 0;
@@ -77,7 +104,16 @@ inline int has_available_input_device(cubeb * ctx)
   }
 
   cubeb_device_collection_destroy(ctx, &devices);
-  return !!input_device_available;
+
+  int backend_has_input_capabilities;
+  const char * backend_id = cubeb_get_backend_id(ctx);
+  for (uint32_t i = 0; i < sizeof(backend_capabilities) / sizeof(backend_caps); i++) {
+    if (strcmp(backend_capabilities[i].id, backend_id) == 0) {
+      backend_has_input_capabilities = backend_capabilities[i].input_capabilities;
+    }
+  }
+
+  return !!input_device_available && !!backend_has_input_capabilities;
 }
 
 inline void print_log(const char * msg, ...)

test/test_callback_ret.cpp

@@ -158,9 +158,9 @@ void run_test_callback(test_direction direction,
     cleanup_cubeb_at_exit(ctx, cubeb_destroy);
 
   if ((direction == INPUT_ONLY || direction == DUPLEX) &&
-      !has_available_input_device(ctx)) {
+      !can_run_audio_input_test(ctx)) {
     /* This test needs an available input device, skip it if this host does not
-    * have one. */
+    * have one or if the backend doesn't implement input. */
     return;
   }
 

test/test_device_changed_callback.cpp

@@ -75,9 +75,14 @@ TEST(cubeb, device_changed_callbacks)
   int r = CUBEB_OK;
   uint32_t latency_frames = 0;
 
+
   r = common_init(&ctx, "Cubeb duplex example with device change");
   ASSERT_EQ(r, CUBEB_OK) << "Error initializing cubeb library";
 
+  if (!can_run_audio_input_test(ctx)) {
+    return;
+  }
+
   std::unique_ptr<cubeb, decltype(&cubeb_destroy)>
     cleanup_cubeb_at_exit(ctx, cubeb_destroy);
 

test/test_devices.cpp

@@ -185,6 +185,9 @@ TEST(cubeb, enumerate_devices)
   count_before_creating_duplex_stream = collection.count;
   cubeb_device_collection_destroy(ctx, &collection);
 
+  if (!can_run_audio_input_test(ctx)) {
+    return;
+  }
   cubeb_stream * stream;
   cubeb_stream_params input_params;
   cubeb_stream_params output_params;
@@ -221,6 +224,10 @@ TEST(cubeb, stream_get_current_device)
   fprintf(stdout, "Getting current devices for backend %s\n",
     cubeb_get_backend_id(ctx));
 
+  if (!can_run_audio_input_test(ctx)) {
+    return;
+  }
+
   cubeb_stream * stream = NULL;
   cubeb_stream_params input_params;
   cubeb_stream_params output_params;
@@ -252,4 +259,4 @@ TEST(cubeb, stream_get_current_device)
 
   r = cubeb_stream_device_destroy(stream, device);
   ASSERT_EQ(r, CUBEB_OK) << "Error destroying current devices";
-}
+}

test/test_duplex.cpp

@@ -96,7 +96,7 @@ TEST(cubeb, duplex)
 
   /* This test needs an available input device, skip it if this host does not
    * have one. */
-  if (!has_available_input_device(ctx)) {
+  if (!can_run_audio_input_test(ctx)) {
     return;
   }
 
@@ -184,6 +184,12 @@ TEST(cubeb, duplex_collection_change)
   std::unique_ptr<cubeb, decltype(&cubeb_destroy)> cleanup_cubeb_at_exit(
       ctx, cubeb_destroy);
 
+  /* This test needs an available input device, skip it if this host does not
+   * have one. */
+  if (!can_run_audio_input_test(ctx)) {
+    return;
+  }
+
   duplex_collection_change_impl(ctx);
   r = cubeb_register_device_collection_changed(
       ctx, static_cast<cubeb_device_type>(CUBEB_DEVICE_TYPE_INPUT), nullptr,
@@ -198,6 +204,14 @@ TEST(cubeb, duplex_collection_change_no_unregister)
 
   r = common_init(&ctx, "Cubeb duplex example with collection change");
   ASSERT_EQ(r, CUBEB_OK) << "Error initializing cubeb library";
+
+  /* This test needs an available input device, skip it if this host does not
+   * have one. */
+  if (!can_run_audio_input_test(ctx)) {
+    cubeb_destroy(ctx);
+    return;
+  }
+
   std::unique_ptr<cubeb, decltype(&cubeb_destroy)> cleanup_cubeb_at_exit(
       ctx, [](cubeb * p) noexcept { EXPECT_DEATH(cubeb_destroy(p), ""); });
 

test/test_loopback.cpp

@@ -267,6 +267,12 @@ void run_loopback_duplex_test(bool is_float)
   std::unique_ptr<cubeb, decltype(&cubeb_destroy)>
     cleanup_cubeb_at_exit(ctx, cubeb_destroy);
 
+  /* This test needs an available input device, skip it if this host does not
+   * have one. */
+  if (!can_run_audio_input_test(ctx)) {
+    return;
+  }
+
   input_params.format = is_float ? CUBEB_SAMPLE_FLOAT32NE : CUBEB_SAMPLE_S16LE;
   input_params.rate = SAMPLE_FREQUENCY;
   input_params.channels = 1;
@@ -340,6 +346,10 @@ void run_loopback_separate_streams_test(bool is_float)
   std::unique_ptr<cubeb, decltype(&cubeb_destroy)>
     cleanup_cubeb_at_exit(ctx, cubeb_destroy);
 
+  if (!can_run_audio_input_test(ctx)) {
+    return;
+  }
+
   input_params.format = is_float ? CUBEB_SAMPLE_FLOAT32NE : CUBEB_SAMPLE_S16LE;
   input_params.rate = SAMPLE_FREQUENCY;
   input_params.channels = 1;
@@ -423,6 +433,10 @@ void run_loopback_silence_test(bool is_float)
   std::unique_ptr<cubeb, decltype(&cubeb_destroy)>
     cleanup_cubeb_at_exit(ctx, cubeb_destroy);
 
+  if (!can_run_audio_input_test(ctx)) {
+    return;
+  }
+
   input_params.format = is_float ? CUBEB_SAMPLE_FLOAT32NE : CUBEB_SAMPLE_S16LE;
   input_params.rate = SAMPLE_FREQUENCY;
   input_params.channels = 1;
@@ -486,6 +500,10 @@ void run_loopback_device_selection_test(bool is_float)
   std::unique_ptr<cubeb, decltype(&cubeb_destroy)>
     cleanup_cubeb_at_exit(ctx, cubeb_destroy);
 
+  if (!can_run_audio_input_test(ctx)) {
+    return;
+  }
+
   r = cubeb_enumerate_devices(ctx, CUBEB_DEVICE_TYPE_OUTPUT, &collection);
   if (r == CUBEB_ERROR_NOT_SUPPORTED) {
     fprintf(stderr, "Device enumeration not supported"

test/test_overload_callback.cpp

@@ -66,6 +66,12 @@ TEST(cubeb, overload_callback)
   std::unique_ptr<cubeb, decltype(&cubeb_destroy)>
     cleanup_cubeb_at_exit(ctx, cubeb_destroy);
 
+  // This test is specifically designed to test a behaviour of the WASAPI
+  // backend in a specific scenario.
+  if (strcmp(cubeb_get_backend_id(ctx), "wasapi") != 0) {
+    return;
+  }
+
   output_params.format = STREAM_FORMAT;
   output_params.rate = 48000;
   output_params.channels = 2;

test/test_record.cpp

@@ -86,7 +86,7 @@ TEST(cubeb, record)
 
   /* This test needs an available input device, skip it if this host does not
    * have one. */
-  if (!has_available_input_device(ctx)) {
+  if (!can_run_audio_input_test(ctx)) {
     return;
   }