directsoundsink: Fix corner case causing large CPU usage
authorNirbheek Chauhan <nirbheek@centricular.com>
Wed, 3 May 2017 17:53:10 +0000 (23:23 +0530)
committerSebastian Dröge <sebastian@centricular.com>
Mon, 8 May 2017 15:15:33 +0000 (17:15 +0200)
We were unnecessarily looping/goto-ing repeatedly when we had exactly
the amount of data as the free space, and also when the free space was
too small. This, as it turns out, is a very common scenario with
Directsound on Windows.

Fixes https://bugzilla.gnome.org/show_bug.cgi?id=773681

We have to do polling here because the event notification API that
Directsound exposes cannot be used with live playback since all events
must be registered in advance with the capture buffer, you cannot
add/remove them once playback has begun. Directsoundsrc had the same
problem.

See also: https://bugzilla.gnome.org/show_bug.cgi?id=781249

sys/directsound/gstdirectsoundsink.c

index 573e2b8..d9cefbd 100644 (file)
@@ -624,10 +624,10 @@ gst_directsound_sink_write (GstAudioSink * asink, gpointer data, guint length)
 
   if (SUCCEEDED (hRes) && SUCCEEDED (hRes2) && (dwStatus & DSBSTATUS_PLAYING)) {
     DWORD dwFreeBufferSize = 0;
-    DWORD sleepTime = 0;
+    guint64 sleep_time_ms = 0;
 
   calculate_freesize:
-    /* calculate the free size of the circular buffer */
+    /* Calculate the free space in the circular buffer */
     if (dwCurrentPlayCursor < dsoundsink->current_circular_offset)
       dwFreeBufferSize =
           dsoundsink->buffer_size - (dsoundsink->current_circular_offset -
@@ -636,21 +636,28 @@ gst_directsound_sink_write (GstAudioSink * asink, gpointer data, guint length)
       dwFreeBufferSize =
           dwCurrentPlayCursor - dsoundsink->current_circular_offset;
 
-    if (length >= dwFreeBufferSize) {
-      sleepTime =
-          ((length -
-              dwFreeBufferSize) * 1000) / (dsoundsink->bytes_per_sample *
-          GST_AUDIO_BASE_SINK (asink)->ringbuffer->spec.info.rate);
-      if (sleepTime > 0) {
-        GST_DEBUG_OBJECT (dsoundsink,
-            "gst_directsound_sink_write: length:%i, FreeBufSiz: %ld, sleepTime: %ld, bps: %i, rate: %i",
-            length, dwFreeBufferSize, sleepTime, dsoundsink->bytes_per_sample,
-            GST_AUDIO_BASE_SINK (asink)->ringbuffer->spec.info.rate);
-        Sleep (sleepTime);
-      }
+    /* Not enough free space, wait for some samples to be played out. We could
+     * write out partial data, but that will result in a tight loop in the
+     * audioringbuffer write thread, and lead to high CPU usage. */
+    if (length > dwFreeBufferSize) {
+      gint rate = GST_AUDIO_BASE_SINK (asink)->ringbuffer->spec.info.rate;
+      /* Wait for a time proportional to the space needed. In reality, the
+       * directsound sink's position does not update frequently enough, so we
+       * will end up waiting for much longer. Note that Sleep() has millisecond
+       * resolution at best. */
+      sleep_time_ms = gst_util_uint64_scale_int ((length - dwFreeBufferSize),
+          1000, dsoundsink->bytes_per_sample * rate);
+      /* Make sure we don't run in a tight loop unnecessarily */
+      sleep_time_ms = MAX (sleep_time_ms, 10);
+      GST_DEBUG_OBJECT (dsoundsink,
+          "length: %u, FreeBufSiz: %ld, sleep_time_ms: %" G_GUINT64_FORMAT
+          ", bps: %i, rate: %i", length, dwFreeBufferSize, sleep_time_ms,
+          dsoundsink->bytes_per_sample, rate);
+      Sleep (sleep_time_ms);
+
+      /* May we send out? */
       hRes = IDirectSoundBuffer_GetCurrentPosition (dsoundsink->pDSBSecondary,
           &dwCurrentPlayCursor, NULL);
-
       hRes2 =
           IDirectSoundBuffer_GetStatus (dsoundsink->pDSBSecondary, &dwStatus);
       if (SUCCEEDED (hRes) && SUCCEEDED (hRes2)
@@ -677,10 +684,10 @@ gst_directsound_sink_write (GstAudioSink * asink, gpointer data, guint length)
 
   if (dwStatus & DSBSTATUS_BUFFERLOST) {
     hRes = IDirectSoundBuffer_Restore (dsoundsink->pDSBSecondary);      /*need a loop waiting the buffer is restored?? */
-
     dsoundsink->current_circular_offset = 0;
   }
 
+  /* Lock a buffer of length @length for writing */
   hRes = IDirectSoundBuffer_Lock (dsoundsink->pDSBSecondary,
       dsoundsink->current_circular_offset, length, &pLockedBuffer1,
       &dwSizeBuffer1, &pLockedBuffer2, &dwSizeBuffer2, 0L);