[WebRTC][GStreamer] Make sure to have the default microphone on the top of the list
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 28 Nov 2018 14:56:11 +0000 (14:56 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 28 Nov 2018 14:56:11 +0000 (14:56 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192026

Patch by Thibault Saunier <tsaunier@igalia.com> on 2018-11-28
Reviewed by Philippe Normand.

Otherwise we might end up picking a useless one in some applications
(not sure what those application do though).

GStreamer patch proposed as https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/merge_requests/34/diffs

Source/WebCore:

* platform/mediastream/gstreamer/GStreamerCaptureDeviceManager.cpp:
(WebCore::sortDevices):
(WebCore::GStreamerCaptureDeviceManager::addDevice):
(WebCore::GStreamerCaptureDeviceManager::refreshCaptureDevices):
* platform/mediastream/gstreamer/GStreamerCaptureDeviceManager.h:

Tools:

* flatpak/org.webkit.CommonModules.yaml:
* gstreamer/jhbuild.modules:
* gstreamer/patches/gst-plugins-good-0014-pulse-Mark-default-devices-as-default.patch: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238609 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/mediastream/gstreamer/GStreamerCaptureDeviceManager.cpp
Source/WebCore/platform/mediastream/gstreamer/GStreamerCaptureDeviceManager.h
Tools/ChangeLog
Tools/flatpak/org.webkit.CommonModules.yaml
Tools/gstreamer/jhbuild.modules
Tools/gstreamer/patches/gst-plugins-good-0014-pulse-Mark-default-devices-as-default.patch [new file with mode: 0644]

index 4b8d9de..eafe559 100644 (file)
@@ -1,5 +1,23 @@
 2018-11-28  Thibault Saunier  <tsaunier@igalia.com>
 
+        [WebRTC][GStreamer] Make sure to have the default microphone on the top of the list
+        https://bugs.webkit.org/show_bug.cgi?id=192026
+
+        Reviewed by Philippe Normand.
+
+        Otherwise we might end up picking a useless one in some applications
+        (not sure what those application do though).
+
+        GStreamer patch proposed as https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/merge_requests/34/diffs
+
+        * platform/mediastream/gstreamer/GStreamerCaptureDeviceManager.cpp:
+        (WebCore::sortDevices):
+        (WebCore::GStreamerCaptureDeviceManager::addDevice):
+        (WebCore::GStreamerCaptureDeviceManager::refreshCaptureDevices):
+        * platform/mediastream/gstreamer/GStreamerCaptureDeviceManager.h:
+
+2018-11-28  Thibault Saunier  <tsaunier@igalia.com>
+
         [WebRTC][GStreamer] Tag all cameras with as 'Unknown' facing mode
         https://bugs.webkit.org/show_bug.cgi?id=192028
 
index a273d44..c7cf33b 100644 (file)
 
 namespace WebCore {
 
+static gint sortDevices(gconstpointer a, gconstpointer b)
+{
+    GstDevice* adev = GST_DEVICE(a), *bdev = GST_DEVICE(b);
+    GUniquePtr<GstStructure> aprops(gst_device_get_properties(adev));
+    GUniquePtr<GstStructure> bprops(gst_device_get_properties(bdev));
+    gboolean aIsDefault = FALSE, bIsDefault = FALSE;
+
+    gst_structure_get_boolean(aprops.get(), "is-default", &aIsDefault);
+    gst_structure_get_boolean(bprops.get(), "is-default", &bIsDefault);
+
+    if (aIsDefault == bIsDefault) {
+        GUniquePtr<char> aName(gst_device_get_display_name(adev));
+        GUniquePtr<char> bName(gst_device_get_display_name(bdev));
+
+        return g_strcmp0(aName.get(), bName.get());
+    }
+
+    return aIsDefault > bIsDefault ? -1 : 1;
+}
+
 GStreamerAudioCaptureDeviceManager& GStreamerAudioCaptureDeviceManager::singleton()
 {
     static NeverDestroyed<GStreamerAudioCaptureDeviceManager> manager;
@@ -66,7 +86,7 @@ const Vector<CaptureDevice>& GStreamerCaptureDeviceManager::captureDevices()
     return m_devices;
 }
 
-void GStreamerCaptureDeviceManager::deviceAdded(GRefPtr<GstDevice>&& device)
+void GStreamerCaptureDeviceManager::addDevice(GRefPtr<GstDevice>&& device)
 {
     GUniquePtr<GstStructure> properties(gst_device_get_properties(device.get()));
     const char* klass = gst_structure_get_string(properties.get(), "device.class");
@@ -85,7 +105,10 @@ void GStreamerCaptureDeviceManager::deviceAdded(GRefPtr<GstDevice>&& device)
     // This isn't really a UID but should be good enough (libwebrtc
     // itself does that at least for pulseaudio devices).
     GUniquePtr<char> deviceName(gst_device_get_display_name(device.get()));
-    String identifier = String::fromUTF8(deviceName.get());
+    gboolean isDefault = FALSE;
+    gst_structure_get_boolean(properties.get(), "is-default", &isDefault);
+
+    String identifier = String::format("%s%s", isDefault ? "default: " : "", deviceName.get());
 
     auto gstCaptureDevice = GStreamerCaptureDevice(WTFMove(device), identifier, type, identifier);
     gstCaptureDevice.setEnabled(true);
@@ -120,10 +143,11 @@ void GStreamerCaptureDeviceManager::refreshCaptureDevices()
         return;
     }
 
-    GList* devices = gst_device_monitor_get_devices(m_deviceMonitor.get());
+    GList* devices = g_list_sort(gst_device_monitor_get_devices(m_deviceMonitor.get()), sortDevices);
     while (devices) {
         GRefPtr<GstDevice> device = adoptGRef(GST_DEVICE_CAST(devices->data));
-        deviceAdded(WTFMove(device));
+
+        addDevice(WTFMove(device));
         devices = g_list_delete_link(devices, devices);
     }
 
index 2a642f3..ce11c4f 100644 (file)
@@ -38,7 +38,7 @@ public:
     virtual CaptureDevice::DeviceType deviceType() = 0;
 
 private:
-    void deviceAdded(GRefPtr<GstDevice>&&);
+    void addDevice(GRefPtr<GstDevice>&&);
     void refreshCaptureDevices();
 
     GRefPtr<GstDeviceMonitor> m_deviceMonitor;
index 78c72fd..2135767 100644 (file)
@@ -1,3 +1,19 @@
+2018-11-28  Thibault Saunier  <tsaunier@igalia.com>
+
+        [WebRTC][GStreamer] Make sure to have the default microphone on the top of the list
+        https://bugs.webkit.org/show_bug.cgi?id=192026
+
+        Reviewed by Philippe Normand.
+
+        Otherwise we might end up picking a useless one in some applications
+        (not sure what those application do though).
+
+        GStreamer patch proposed as https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/merge_requests/34/diffs
+
+        * flatpak/org.webkit.CommonModules.yaml:
+        * gstreamer/jhbuild.modules:
+        * gstreamer/patches/gst-plugins-good-0014-pulse-Mark-default-devices-as-default.patch: Added.
+
 2018-11-28  Tomas Popela  <tpopela@redhat.com>
 
         [GTK] Silence compilation warnings in glib unittests
index 6bf7d86..247596e 100644 (file)
       path: ../gstreamer/patches/gst-plugins-good-0012-matroskdemux-do-not-use-MapInfo.data-after-unmapping.patch
     - type: patch
       path: ../gstreamer/patches/gst-plugins-good-0013-Avoid-warning-when-reporting-about-decryptors.patch
+    - type: patch
+      path: ../gstreamer/patches/gst-plugins-good-0014-pulse-Mark-default-devices-as-default.patch
   config-opts:
     - -Ddisable_gtkdoc=true
 - name: x264
index 60de019..c7cd4d9 100644 (file)
@@ -90,6 +90,7 @@
       <patch file="gst-plugins-good-0011-matroska-Add-the-WebM-encrypted-content-support-in-m.patch" strip="1" /> <!-- Merged as 0432826950d4d80fe2b50ffd3757dc08155de9e3 -->
       <patch file="gst-plugins-good-0012-matroskdemux-do-not-use-MapInfo.data-after-unmapping.patch" strip="1" /> <!-- Merged as defae350358660e557c74e41a4fe8a8bb327e9c8 -->
       <patch file="gst-plugins-good-0013-Avoid-warning-when-reporting-about-decryptors.patch" strip="1" /> <!-- Merged as 56669205eb2d0887596574eabb7806b31c5ba5cf -->
+      <patch file="gst-plugins-good-0014-pulse-Mark-default-devices-as-default.patch" strip="1" />
     </branch>
   </autotools>
 
diff --git a/Tools/gstreamer/patches/gst-plugins-good-0014-pulse-Mark-default-devices-as-default.patch b/Tools/gstreamer/patches/gst-plugins-good-0014-pulse-Mark-default-devices-as-default.patch
new file mode 100644 (file)
index 0000000..435b6d7
--- /dev/null
@@ -0,0 +1,318 @@
+From 32c833ebb42b8e8f46c5a72a16a874bc00fc5553 Mon Sep 17 00:00:00 2001
+From: Thibault Saunier <tsaunier@igalia.com>
+Date: Mon, 26 Nov 2018 13:48:56 -0300
+Subject: [PATCH] pulse: Mark default devices as "default"
+
+---
+ ext/pulse/pulsedeviceprovider.c | 117 +++++++++++++++++++++-----------
+ ext/pulse/pulsedeviceprovider.h |   3 +
+ 2 files changed, 79 insertions(+), 41 deletions(-)
+
+diff --git a/ext/pulse/pulsedeviceprovider.c b/ext/pulse/pulsedeviceprovider.c
+index a1964dab2..9b66a0e33 100644
+--- a/ext/pulse/pulsedeviceprovider.c
++++ b/ext/pulse/pulsedeviceprovider.c
+@@ -40,7 +40,7 @@ GST_DEBUG_CATEGORY_EXTERN (pulse_debug);
+ static GstDevice *gst_pulse_device_new (guint id,
+     const gchar * device_name, GstCaps * caps, const gchar * internal_name,
+-    GstPulseDeviceType type, GstStructure * properties);
++    GstPulseDeviceType type, GstStructure * properties, gboolean is_default);
+ G_DEFINE_TYPE (GstPulseDeviceProvider, gst_pulse_device_provider,
+     GST_TYPE_DEVICE_PROVIDER);
+@@ -65,6 +65,12 @@ enum
+ };
++typedef struct
++{
++  GList *devices;
++  GstPulseDeviceProvider *self;
++} ListDevicesData;
++
+ static void
+ gst_pulse_device_provider_class_init (GstPulseDeviceProviderClass * klass)
+ {
+@@ -114,6 +120,8 @@ gst_pulse_device_provider_finalize (GObject * object)
+   g_free (self->client_name);
+   g_free (self->server);
++  g_free (self->default_sink_name);
++  g_free (self->default_source_name);
+   G_OBJECT_CLASS (gst_pulse_device_provider_parent_class)->finalize (object);
+ }
+@@ -186,7 +194,7 @@ context_state_cb (pa_context * c, void *userdata)
+ }
+ static GstDevice *
+-new_source (const pa_source_info * info)
++new_source (GstPulseDeviceProvider * self, const pa_source_info * info)
+ {
+   GstCaps *caps;
+   GstStructure *props;
+@@ -200,11 +208,12 @@ new_source (const pa_source_info * info)
+   props = gst_pulse_make_structure (info->proplist);
+   return gst_pulse_device_new (info->index, info->description,
+-      caps, info->name, GST_PULSE_DEVICE_TYPE_SOURCE, props);
++      caps, info->name, GST_PULSE_DEVICE_TYPE_SOURCE, props,
++      !g_strcmp0 (info->name, self->default_source_name));
+ }
+ static GstDevice *
+-new_sink (const pa_sink_info * info)
++new_sink (GstPulseDeviceProvider * self, const pa_sink_info * info)
+ {
+   GstCaps *caps;
+   GstStructure *props;
+@@ -218,7 +227,8 @@ new_sink (const pa_sink_info * info)
+   props = gst_pulse_make_structure (info->proplist);
+   return gst_pulse_device_new (info->index, info->description,
+-      caps, info->name, GST_PULSE_DEVICE_TYPE_SINK, props);
++      caps, info->name, GST_PULSE_DEVICE_TYPE_SINK, props,
++      !g_strcmp0 (info->name, self->default_sink_name));
+ }
+ static void
+@@ -233,12 +243,26 @@ get_source_info_cb (pa_context * context,
+     return;
+   }
+-  dev = new_source (info);
++  dev = new_source (self, info);
+   if (dev)
+     gst_device_provider_device_add (GST_DEVICE_PROVIDER (self), dev);
+ }
++static void
++get_server_info_cb (pa_context * context, const pa_server_info * info,
++    void *userdata)
++{
++  GstPulseDeviceProvider *self = userdata;
++
++  g_free (self->default_sink_name);
++  g_free (self->default_source_name);
++  self->default_sink_name = g_strdup (info->default_sink_name);
++  self->default_source_name = g_strdup (info->default_source_name);
++
++  pa_threaded_mainloop_signal (self->mainloop, 0);
++}
++
+ static void
+ get_sink_info_cb (pa_context * context,
+     const pa_sink_info * info, int eol, void *userdata)
+@@ -251,7 +275,7 @@ get_sink_info_cb (pa_context * context,
+     return;
+   }
+-  dev = new_sink (info);
++  dev = new_sink (self, info);
+   if (dev)
+     gst_device_provider_device_add (GST_DEVICE_PROVIDER (self), dev);
+@@ -312,34 +336,38 @@ static void
+ get_source_info_list_cb (pa_context * context, const pa_source_info * info,
+     int eol, void *userdata)
+ {
+-  GList **devices = userdata;
++  ListDevicesData *data = userdata;
+   if (eol)
+     return;
+-  *devices = g_list_prepend (*devices, gst_object_ref_sink (new_source (info)));
++  data->devices =
++      g_list_prepend (data->devices,
++      gst_object_ref_sink (new_source (data->self, info)));
+ }
+ static void
+ get_sink_info_list_cb (pa_context * context, const pa_sink_info * info,
+     int eol, void *userdata)
+ {
+-  GList **devices = userdata;
++  ListDevicesData *data = userdata;
+   if (eol)
+     return;
+-  *devices = g_list_prepend (*devices, gst_object_ref_sink (new_sink (info)));
++  data->devices =
++      g_list_prepend (data->devices, gst_object_ref_sink (new_sink (data->self,
++              info)));
+ }
+ static GList *
+ gst_pulse_device_provider_probe (GstDeviceProvider * provider)
+ {
+   GstPulseDeviceProvider *self = GST_PULSE_DEVICE_PROVIDER (provider);
+-  GList *devices = NULL;
+   pa_mainloop *m = NULL;
+   pa_context *c = NULL;
+   pa_operation *o;
++  ListDevicesData data = { NULL, self };
+   if (!(m = pa_mainloop_new ()))
+     return NULL;
+@@ -376,7 +404,7 @@ gst_pulse_device_provider_probe (GstDeviceProvider * provider)
+   }
+   GST_DEBUG_OBJECT (self, "connected");
+-  o = pa_context_get_sink_info_list (c, get_sink_info_list_cb, &devices);
++  o = pa_context_get_sink_info_list (c, get_sink_info_list_cb, &data);
+   while (pa_operation_get_state (o) == PA_OPERATION_RUNNING &&
+       pa_operation_get_state (o) == PA_OPERATION_RUNNING) {
+     if (pa_mainloop_iterate (m, TRUE, NULL) < 0)
+@@ -384,7 +412,7 @@ gst_pulse_device_provider_probe (GstDeviceProvider * provider)
+   }
+   pa_operation_unref (o);
+-  o = pa_context_get_source_info_list (c, get_source_info_list_cb, &devices);
++  o = pa_context_get_source_info_list (c, get_source_info_list_cb, &data);
+   while (pa_operation_get_state (o) == PA_OPERATION_RUNNING &&
+       pa_operation_get_state (o) == PA_OPERATION_RUNNING) {
+     if (pa_mainloop_iterate (m, TRUE, NULL) < 0)
+@@ -395,18 +423,38 @@ gst_pulse_device_provider_probe (GstDeviceProvider * provider)
+   pa_context_disconnect (c);
+   pa_mainloop_free (m);
+-  return devices;
++  return data.devices;
+ failed:
+   return NULL;
+ }
++static gboolean
++run_pulse_operation (GstPulseDeviceProvider * self, pa_operation * operation)
++{
++  if (!operation)
++    return FALSE;
++
++  while (pa_operation_get_state (operation) == PA_OPERATION_RUNNING) {
++    if (!PA_CONTEXT_IS_GOOD (pa_context_get_state ((self->context)))) {
++      pa_operation_cancel (operation);
++      pa_operation_unref (operation);
++      return FALSE;
++    }
++
++    pa_threaded_mainloop_wait (self->mainloop);
++  }
++
++  pa_operation_unref (operation);
++
++  return TRUE;
++}
++
+ static gboolean
+ gst_pulse_device_provider_start (GstDeviceProvider * provider)
+ {
+   GstPulseDeviceProvider *self = GST_PULSE_DEVICE_PROVIDER (provider);
+-  pa_operation *initial_operation;
+   if (!(self->mainloop = pa_threaded_mainloop_new ())) {
+     GST_ERROR_OBJECT (self, "Could not create pulseaudio mainloop");
+@@ -462,27 +510,18 @@ gst_pulse_device_provider_start (GstDeviceProvider * provider)
+   pa_context_subscribe (self->context,
+       PA_SUBSCRIPTION_MASK_SOURCE | PA_SUBSCRIPTION_MASK_SINK, NULL, NULL);
+-  initial_operation = pa_context_get_source_info_list (self->context,
+-      get_source_info_cb, self);
+-  while (pa_operation_get_state (initial_operation) == PA_OPERATION_RUNNING) {
+-    if (!PA_CONTEXT_IS_GOOD (pa_context_get_state ((self->context))))
+-      goto cancel_and_fail;
+-
+-    pa_threaded_mainloop_wait (self->mainloop);
+-  }
+-  pa_operation_unref (initial_operation);
++  if (!run_pulse_operation (self, pa_context_get_server_info (self->context,
++              get_server_info_cb, self)))
++    goto unlock_and_fail;
+-  initial_operation = pa_context_get_sink_info_list (self->context,
+-      get_sink_info_cb, self);
+-  if (!initial_operation)
++  if (!run_pulse_operation (self,
++          pa_context_get_source_info_list (self->context, get_source_info_cb,
++              self)))
+     goto unlock_and_fail;
+-  while (pa_operation_get_state (initial_operation) == PA_OPERATION_RUNNING) {
+-    if (!PA_CONTEXT_IS_GOOD (pa_context_get_state ((self->context))))
+-      goto cancel_and_fail;
+-    pa_threaded_mainloop_wait (self->mainloop);
+-  }
+-  pa_operation_unref (initial_operation);
++  if (!run_pulse_operation (self, pa_context_get_sink_info_list (self->context,
++              get_sink_info_cb, self)))
++    goto unlock_and_fail;
+   pa_threaded_mainloop_unlock (self->mainloop);
+@@ -495,11 +534,6 @@ unlock_and_fail:
+ mainloop_failed:
+   return FALSE;
+-
+-cancel_and_fail:
+-  pa_operation_cancel (initial_operation);
+-  pa_operation_unref (initial_operation);
+-  goto unlock_and_fail;
+ }
+ static void
+@@ -611,7 +645,7 @@ gst_pulse_device_reconfigure_element (GstDevice * device, GstElement * element)
+ static GstDevice *
+ gst_pulse_device_new (guint device_index, const gchar * device_name,
+     GstCaps * caps, const gchar * internal_name, GstPulseDeviceType type,
+-    GstStructure * props)
++    GstStructure * props, gboolean is_default)
+ {
+   GstPulseDevice *gstdev;
+   const gchar *element = NULL;
+@@ -636,7 +670,7 @@ gst_pulse_device_new (guint device_index, const gchar * device_name,
+       break;
+   }
+-
++  gst_structure_set (props, "is-default", G_TYPE_BOOLEAN, is_default, NULL);
+   gstdev = g_object_new (GST_TYPE_PULSE_DEVICE,
+       "display-name", device_name, "caps", caps, "device-class", klass,
+       "internal-name", internal_name, "properties", props, NULL);
+@@ -644,6 +678,7 @@ gst_pulse_device_new (guint device_index, const gchar * device_name,
+   gstdev->type = type;
+   gstdev->device_index = device_index;
+   gstdev->element = element;
++  gstdev->is_default = is_default;
+   gst_structure_free (props);
+   gst_caps_unref (caps);
+diff --git a/ext/pulse/pulsedeviceprovider.h b/ext/pulse/pulsedeviceprovider.h
+index 0892ad586..7bcd1bc47 100644
+--- a/ext/pulse/pulsedeviceprovider.h
++++ b/ext/pulse/pulsedeviceprovider.h
+@@ -50,6 +50,8 @@ struct _GstPulseDeviceProvider {
+   gchar *server;
+   gchar *client_name;
++  gchar *default_source_name;
++  gchar *default_sink_name;
+   pa_threaded_mainloop *mainloop;
+   pa_context *context;
+@@ -84,6 +86,7 @@ struct _GstPulseDevice {
+   GstPulseDeviceType type;
+   guint             device_index;
+   gchar            *internal_name;
++  gboolean         is_default;
+   const gchar      *element;
+ };
+-- 
+2.19.1
+