[Bug 792897] wasapi: Implement multichannel support and a device provider

[GStreamer (GNOME Bugzilla)]

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

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #367418|none                        |needs-work
             status|                            |

--- Comment #3 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 367418:
 --> (https://bugzilla.gnome.org/review?bug=792897&attachment=367418)

Mostly looks good

::: sys/wasapi/gstwasapisink.c
@@ +182,3 @@
   }

+  g_clear_pointer (&self->positions, g_free);

You could also make that a static 64-element array fwiw

::: sys/wasapi/gstwasapiutil.c
@@ +212,3 @@

+static IMMDeviceEnumerator*
+gst_wasapi_util_get_device_enumerator (GstElement * element)

This and the related changes look like something that should've been in the
next commit?

@@ +423,3 @@
+    GST_INFO ("wasapi: got %i channels from wasapi, assuming non-positional",
+        nChannels);
+    mask = GST_AUDIO_CHANNEL_POSITION_NONE;

mask = NONE makes no sense. All positions NONE would be a mask of 0

@@ +430,3 @@
+   * i.e. the speaker positions and the enum values are the same till
+   * 0x100 = SPEAKER_BACK_CENTER = GST_AUDIO_CHANNEL_POSITION_REAR_CENTER
+   * Hence if those are the only bits set, we can just use the mask directly.
*/

Considering the code further below, it might be easiest to just define a
mapping table between WASAPI and GStreamer, and use that for all cases. Now you
special-case <= 9 channels in a weird way

@@ +444,3 @@
+          "mask (%x), assuming non-positional channels, please report a bug",
+          nChannels, (guint) dwChannelMask);
+      mask = GST_AUDIO_CHANNEL_POSITION_NONE;

And here

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.