After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 797345 - wasapisrc - does not drain devices of buffers when reading leading to dropped audio
wasapisrc - does not drain devices of buffers when reading leading to dropped...
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Windows
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-10-27 16:10 UTC by Florian Nierhaus
Modified: 2018-11-03 14:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch for wasapi against master (8.75 KB, patch)
2018-10-27 16:10 UTC, Florian Nierhaus
reviewed Details | Review

Description Florian Nierhaus 2018-10-27 16:10:48 UTC
Created attachment 374068 [details] [review]
patch for wasapi against master

There are a few issues with the buffer handling:

* we sometimes get notified when smaller number of frames are provided by the driver - in this case we need to buffer and wait so gstreamer gets the expected size
* we sometimes get multiple buffers with one notification and we need to save that and return that the next cycle as to not have audio gaps - https://blogs.msdn.microsoft.com/matthew_van_eerde/2014/11/05/draining-the-wasapi-capture-buffer-fully/
* not clearing the audio buffer on some devices on the notification / partial sizes confuses some devices
Comment 1 Nirbheek Chauhan 2018-10-27 16:45:36 UTC
Review of attachment 374068 [details] [review]:

Looks good to me overall, can be merged after these comments are addressed. Thanks, this should improve things a lot!

::: sys/wasapi/gstwasapisrc.c
@@ +477,1 @@
 

The + 1 here means we now use a minimum of 3 segments? Why is that? The comment would need to be updated with that explanation.

@@ +598,3 @@
+      self->overflow_buffer_length -= n;
+    }
+  }

I think it's probably simpler to use GstAdapter here instead of managing memory by hand. It was made specifically for this use-case.

@@ +650,3 @@
+      if ((flags & mask_handled) != 0) {
+        GST_INFO_OBJECT (self, "buffer flags=%#08x", (guint) flags);
+      }

This should probably be split into a separate patch?

@@ +669,3 @@
+      } else {
+        memcpy (data_ptr, from, read_len);
+      }

This should also be a separate patch? To write zeroes for silence.
Comment 2 GStreamer system administrator 2018-11-03 14:37:05 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/808.